[ovs-discuss] suspicious usage of fchmod

Ben Pfaff blp at nicira.com
Thu Apr 21 16:23:17 UTC 2011


On Wed, Apr 20, 2011 at 02:55:10PM -0700, Jesse Gross wrote:
> On Wed, Apr 20, 2011 at 8:58 AM, Ben Pfaff <blp at nicira.com> wrote:
> > On Tue, Apr 19, 2011 at 06:50:44PM -0700, Jesse Gross wrote:
> >> On Mon, Apr 18, 2011 at 11:25 AM, Ben Pfaff <blp at nicira.com> wrote:
> >> > With all that taken into account, here is a new version. ?On Linux, I
> >> > prefer the fchmod() based solution, because someday OVS may have to
> >> > add support for threads and the umask is not thread-local.
> >>
> >> Doesn't this just introduce another portability problem? ?If we do
> >> introduce threads then it will be less likely that we notice it since
> >> things will work fine on Linux.
> >
> > We'll have to do extensive grepping around at that point anyway and
> > the system-specific #if should stand out well enough.
> 
> But what's the point of introducing this #ifdef in the first place?
> It seems like there are two situations:
> * We don't introduce threading, in which case there is no problem but
> also no benefit from this extra piece of OS dependent code which will
> rarely get exercised.
> * We do implement threading, in which case there is a piece of code
> which we will hopefully remember or find that needs to be fixed.

It sounds like you'd prefer to use the portable solution everywhere.
Fine.  Please review this:

--8<--------------------------cut here-------------------------->8--

From: Ben Pfaff <blp at nicira.com>
Date: Thu, 21 Apr 2011 09:22:39 -0700
Subject: [PATCH] socket-util: Use portable solution for setting Unix socket permissions.

Requested-by: Jesse Gross <jesse at nicira.com>
---
 lib/socket-util.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/lib/socket-util.c b/lib/socket-util.c
index 7e4b8be..24e8f81 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -306,18 +306,11 @@ make_sockaddr_un(const char *name, struct sockaddr_un *un, socklen_t *un_len,
 static int
 bind_unix_socket(int fd, struct sockaddr *sun, socklen_t sun_len)
 {
-#ifdef __linux__
-    /* On Linux, calling fchmod() *before* bind() sets permissions for the file
-     * about to be created.  Calling fchmod() *after* bind has no effect on the
-     * file that was created.) */
-    return fchmod(fd, 0700) || bind(fd, sun, sun_len) ? errno : 0;
-#else
     /* According to _Unix Network Programming_, umask should affect bind(). */
     mode_t old_umask = umask(0077);
     int error = bind(fd, sun, sun_len) ? errno : 0;
     umask(old_umask);
     return error;
-#endif
 }
 
 /* Creates a Unix domain socket in the given 'style' (either SOCK_DGRAM or
-- 
1.7.1




More information about the discuss mailing list