Re: [Gluster-devel] Problem with with fuse_setattr()

[Filipe Maia]

Any developments on this bug? I know the example it's kind of a corner
case but this is gonna came back to haunt us sometime.

On Fri, Jan 23, 2009 at 22:06, Filipe Maia <address@hidden> wrote:
> The following example causes the bug.
>
> Using root create a file with permissions 6555.
> Then chown to some other user.
>
> touch goo
> chmod 6555 goo
> chown 65534.65533 goo
> stat goo
>  You will see that the chown will not work. This is because when you
> issue the chown fuse_setattr will be
> called with the valid == 7 because the chown also has to remove the
> setgid and setguid bits.
>
> In summary the chown will only remove the setgid and setuid bits
> changing the permissions to 0555.
> Then if you call chown again you will get the correct result.
>
> So yes this actually happens in practise.
>
>> On Jan 23, 2009 7:48 AM, "Filipe Maia" <address@hidden> wrote:
>>
>> Hi,
>>
>> There is a problem with fuse_setattr() when the valid flag matches
>> more than one condition.
>> Imagine the following situation:
>>
>> A file is created by the root and it's mode changed to 6555. Then the
>> owner is change to someone else.
>> When  this happens the setuid and setgid bits are removed at the same
>> as the owner is changed.
>> In this case valid == FUSE_SET_ATTR_MODE|FUSE_SET_ATTR_UID |
>> FUSE_SET_ATTR_GID.
>>
>> Due to the if else structure of the code the chown call will only
>> change the permissions it will not actually change the owner. Only the
>> second chown call will change the owner.
>> Changing the if else to a bunch of ifs causes problem which I think
>> are related to double freeing of some variables. It seems to me that
>> with the current code it's difficult to do
>> two fops from the same setattr.
>>
>> The best solution I found for now was to use the following patch:
>>
>> --- fuse-bridge.c.old   2009-01-23 16:39:32.000000000 +0100
>> +++ fuse-bridge.c       2009-01-23 16:38:43.000000000 +0100
>> @@ -858,11 +858,10 @@
>>               int valid,
>>               struct fuse_file_info *fi)
>>  {
>> -
>> -        if (valid & FUSE_SET_ATTR_MODE)
>> -                do_chmod (req, ino, attr, fi);
>> -        else if (valid & (FUSE_SET_ATTR_UID | FUSE_SET_ATTR_GID))
>> +        if (valid & (FUSE_SET_ATTR_UID | FUSE_SET_ATTR_GID))
>>                 do_chown (req, ino, attr, valid, fi);
>> +        else if (valid & FUSE_SET_ATTR_MODE)
>> +                do_chmod (req, ino, attr, fi);
>>         else if (valid & FUSE_SET_ATTR_SIZE)
>>                 do_truncate (req, ino, attr, fi);
>>         else if (valid & (FUSE_SET_ATTR_ATIME | FUSE_SET_ATTR_MTIME))
>>
>> Having the USE_SET_ATTR_UID | FUSE_SET_ATTR_GID tested before
>> FUSE_SET_ATTR_MOD will cause the correct behaviour with the chown on
>> my machine.
>> But hardly seems like a proper solution. The best way I think would be
>> to test all possible flags one at a time, but I didn't manage to get
>> that working.
>>
>> Filipe
>>
>>
>> _______________________________________________
>> Gluster-devel mailing list
>> address@hidden
>> http://lists.nongnu.org/mailman/listinfo/gluster-devel
>>
>