[Gluster-devel] Problem with with fuse_setattr()

[Filipe Maia]

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" <filipe at xray.bmc.uu.se> 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
> Gluster-devel at nongnu.org
> http://lists.nongnu.org/mailman/listinfo/gluster-devel
>