[Gluster-devel] 3.5.1-beta2 Problems with suid and sgid bits on directories

[Anders Blomdell]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2014-06-25 22:41, Shyamsundar Ranganathan wrote:
> Hi Anders,
> 
> There are multiple problems that I see in the test provided, here is
> answering one of them and the reason why this occurs. It does get
> into the code and functions a bit, but bottom line is that on a code
> path the setattr that DHT does, misses setting the SGID bit causing
> the problem observed.
> 
> - When directory is healed on a newly added brick it loses the SGID
> mode bit
> 
> This is happening due to 2 reasons,
> 
> mkdir does not honor the SGID mode bit [1]. So when initially
> creating the directory when there is a single brick, an strace of the
> mkdir command shows an fchmod which actually changes the mode of the
> file to add the SGID bit to it.
> 
> In DHT we get into dht_lookup_dir_cbk, as a part of the lookup when
> creating the new directory .../dir2, as the graph has changed due to
> a brick addition (otherwise we would have gone into revalidate path
> where the previous fix was made). Here we call the function,
> dht_selfheal_directory which would create the missing directories,
> with the expected attributes.
> 
> DHT winds a call to mkdir as a part of the dht_selfheal_directory (in
> dht_selfheal_dir_mkdir where it winds a call to mkdir for all
> subvolumes that have the directory missing) with the right mode bits
> (in this case with the SGID bit). As the POSIX layer on the brick
> calls mkdir, the SGID bit is not set for the newly created directory
> due to [1].
I think this depends on the sgid bit of the parent directory on the brick,
which might indicate that mkdir_p should be checked as well.

> Further to calling mkdir DHT now winds an setattr to set the mode
> bits straight, but ends up using the mode bits that are returned in
> the iatt (stat) information by the just concluded mkdir wind, which
> has the SGID bit missing, as mkdir returns the stat information from
> posix_mkdir, by doing a stat post mkdir. Hence we never end up
> setting the SGID bit in the setattr part of DHT.
To me this does not quite explain how a directory (sometimes) winds up with
permissions set to 0.

> 
> Rectification of the problem would be in (need to close out some more
> analysis) dht_selfheal_dir_mkdir_cbk, where we need to pass to the
> subsequent dht_selfheal_dir_setattr the right mode bits to set on the
> directories.
> 
> I will provide a patch for the above issue, post testing out the same
> with the provided script, possibly tomorrow. 
I would be very much obliged for this.

> This would make the
> directory equal on all the bricks, and further discrepancies from the
> mount point or on the backed should not be seen.
Make sure to use the last version (currently 3) of the test script from 
https://bugzilla.redhat.com/show_bug.cgi?id=1110262

> One of the other problems seems to stem from which stat information
> we pick in DHT to return for the mount, the above fix would take care
> of that issue as well, but still something that needs some
> understanding and possible correction.
> 
> [1] see NOTES in, man 2 mkdir
> 
> Shyam
> 
Thanks

/Anders

- -- 
Anders Blomdell                  Email: anders.blomdell at control.lth.se
Department of Automatic Control
Lund University                  Phone:    +46 46 222 4625
P.O. Box 118                     Fax:      +46 46 138118
SE-221 00 Lund, Sweden

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTq842AAoJENZYyvaDG8NcxYwH/j0JVcchfdR6RllE7nrB0LSR
SmjjYxWqr/dg+897jQ0SnIl8SHsI4HCqywtctnk9q3n2sDNG+N6GJQeWXdCycfAs
QZW9wcyDmyqt4FD4kivFtuX4yzHTO1T6Y5G/CMSl6JWhY6gQd9GuPH3t3l31u/43
PuSS4Sey2coaQnKidMVMlCPvLwkzNQtPOgaN7081PUvQu6Ab0IAuuBg2diHwwvPH
RbYmBwVbCgAtd1MP7cfbiCnKy3EKUMSXYzgzoCptyTNy/PTIlysOl8bgWTSpN6AW
sb0j/dz4BRRK/i6IXtpxX1Tf/j9tmYSAG7e3OvavXCqt2m6imh4/ZUIM6h95KN4=
=S77i
-----END PGP SIGNATURE-----