Bug 487108

+++ This bug was initially created as a clone of Bug #485099 +++

There is an inconsistency seen in the behaviour of ext3 and nfs  when changing owner or group of a directory. If the directory has SUID/SGID flags set, the flags are maintained on ext3 however they are stripped on nfs.

Reproducer:

ext3:

file
# touch xyz; chmod u+s xyz; ls -l xyz; chown bin xyz; ls -l xyz
-rwSr--r--  1 root root 0 Feb 10 16:25 xyz
-rw-r--r--  1 bin root 0 Feb 10 16:25 xyz

directory
mkdir abc; chmod u+s abc; ls -ld abc; chown bin abc; ls -ld abc
drwsr-xr-x  2 root root 4096 Feb 10 16:23 abc
drwsr-xr-x  2 bin root 4096 Feb 10 16:23 abc


nfs:

file
# touch xyz; chmod u+s xyz; ls -l xyz; chown bin xyz; ls -l xyz
-rwSr--r--  1 root root 0 Feb 10 16:25 xyz
-rw-r--r--  1 bin root 0 Feb 10 16:25 xyz

directory
# mkdir abc; chmod u+s abc; ls -ld abc; chown bin abc; ls -ld abc
drwsr-xr-x  2 root root 4096 Feb 10 16:23 abc
drwxr-xr-x  2 bin root 4096 Feb 10 16:23 abc

Check the SUID/SGID flags on the directory before and after the chown operation for directories. The flags are stripped under nfs but not under ext3. The behaviour for operations on the file are consistent.

--- Additional comment from sprabhu on 2009-02-11 11:13:15 EDT ---

The behaviour as it stands does not appear to violate POSIX. See:

http://www.opengroup.org/onlinepubs/7990989775/xcu/chgrp.html
http://www.opengroup.org/onlinepubs/7990989775/xsh/chown.html

"Unless chgrp is invoked by a process with appropriate privileges, the set-user-ID and set-group-ID bits of a regular file will be cleared upon successful completion; the set-user-ID and set-group-ID bits of other file types may be cleared."

"If the path argument refers to a regular file, the set-user-ID (S_ISUID) and set-group-ID (S_ISGID) bits of the file mode are cleared upon successful return from chown(), unless the call is made by a process with appropriate privileges, in which case it is implementation-dependent whether these bits are altered. If chown() is successfully invoked on a file that is not a regular file, these bits may be cleared. These bits are defined in <sys/stat.h>."

So POSIX does not require a particular behaviour even if the process has "appropriate privileges" (which itself is not defined) but leaves this to be defined by the implementation. However the actions performed are inconsistent when comparing ext3 and nfs.

--- Additional comment from jlayton on 2009-02-11 11:34:34 EDT ---

Yes, even if we don't violate the letter of the spec it would be good to have the kernel behave consistently here. The point is that chown_common (which is what chown() calls from userspace pass through) only tries to kill the setuid/gid bits if it's not a directory.

I think we should add a clause to nfsd_setattr that just does the bit-clearing if !S_ISDIR.

This will need to go upstream first, but it may be a little while before I can get to it. Shouldn't be too hard a patch if someone else wants to do the legwork for it though.

--- Additional comment from sprabhu on 2009-02-23 11:39:26 EDT ---

http://article.gmane.org/gmane.linux.nfs/24652