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

[Shyamsundar Ranganathan]

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].

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.

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

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