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