[ovs-discuss] suspicious usage of fchmod

Ben Pfaff blp at nicira.com
Fri Apr 15 17:02:47 UTC 2011


On Fri, Apr 15, 2011 at 11:54:14AM +0900, YAMAMOTO Takashi wrote:
> fchown on a socket is not portable.
> 
> i don't think it makes sense even on linux.  (not confirmed)
> 
> YAMAMOTO Takashi
> 
> 
> diff --git a/lib/socket-util.c b/lib/socket-util.c
> index 12bbc71..2b75dee 100644
> --- a/lib/socket-util.c
> +++ b/lib/socket-util.c
> @@ -349,7 +349,7 @@ make_unix_socket(int style, bool nonblock, bool passcred OVS_UNUSED,
>  
>          error = make_sockaddr_un(bind_path, &un, &un_len, &dirfd);
>          if (!error && (bind(fd, (struct sockaddr*) &un, un_len)
> -                       || fchmod(fd, S_IRWXU))) {
> +                       || chmod(bind_path, S_IRWXU))) {
>              error = errno;
>          }
>          if (dirfd >= 0) {

That change introduces a race.

Here's a fix that I have tested to work.  What do you think?

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

From: Ben Pfaff <blp at nicira.com>
Date: Fri, 15 Apr 2011 10:01:56 -0700
Subject: [PATCH] socket-util: Properly set socket permissions in make_unix_socket().

Under Linux, at least, bind and fchmod interact for Unix sockets in a way
that surprised me.  Calling fchmod() on a Unix socket successfully sets the
permissions for the socket's own inode.  But that has no effect on any
inode that has already been created in the file system by bind(), because
that inode is not the same as the one for the Unix socket itself.

However, if you bind() *after* calling fchmod(), then the bind() takes the
permissions for the new inode from the Unix socket inode, which has the
desired effect.

Reported-by: YAMAMOTO Takashi <yamamoto at valinux.co.jp>
---
 lib/socket-util.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/socket-util.c b/lib/socket-util.c
index 12bbc71..fab3cbc 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -348,8 +348,8 @@ make_unix_socket(int style, bool nonblock, bool passcred OVS_UNUSED,
         fatal_signal_add_file_to_unlink(bind_path);
 
         error = make_sockaddr_un(bind_path, &un, &un_len, &dirfd);
-        if (!error && (bind(fd, (struct sockaddr*) &un, un_len)
-                       || fchmod(fd, S_IRWXU))) {
+        if (!error && (fchmod(fd, S_IRWXU)
+                       || bind(fd, (struct sockaddr*) &un, un_len))) {
             error = errno;
         }
         if (dirfd >= 0) {
-- 
1.7.1




More information about the discuss mailing list