hi,

> 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?

is this piece of code intended to be linux-only?
even if so, i don't think relying on this rather undocumented behaviour
is a good idea.

i think a common way to solve this kind of race is to create a directory
with a desired permission and then bind in the directory.

(i don't know if this race can really be a problem because i'm not
familiar with this code.)

YAMAMOTO Takashi

> 
> --8<--------------------------cut here-------------------------->8--
> 
> From: Ben Pfaff <[email protected]>
> 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 <[email protected]>
> ---
>  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
_______________________________________________
discuss mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/discuss

Reply via email to