On 2014/4/18 21:02, J. Bruce Fields wrote:
On Sun, Apr 13, 2014 at 11:11:39PM +0800, Kinglong Mee wrote:
As local filesystem, writing data to the file by non-owner will
clears the SUID+SGID, owner will not.
Are you sure about this? (Do you have a test case that fails?)
Sorry, maybe there are some fault of the comment,
and also, there are some problems needs rechecking.
Please ignore this patch.
I test it using command line with (root, local ext4),
touch test; chmod 4777 test; stat test; echo 1234 > test; stat test;
File: ‘test’
Size: 0 Blocks: 0 IO Block: 4096 regular
empty file
Device: 803h/2051d Inode: 1200137 Links: 1
Access: (4777/-rwsrwxrwx) Uid: ( 0/ root) Gid: ( 0/ root)
Context: unconfined_u:object_r:admin_home_t:s0
Access: 2014-04-18 21:36:31.016029014 +0800
Modify: 2014-04-18 21:36:31.016029014 +0800
Change: 2014-04-18 21:36:31.026030285 +0800
Birth: -
File: ‘test’
Size: 5 Blocks: 8 IO Block: 4096 regular file
Device: 803h/2051d Inode: 1200137 Links: 1
Access: (4777/-rwsrwxrwx) Uid: ( 0/ root) Gid: ( 0/ root)
Context: unconfined_u:object_r:admin_home_t:s0
Access: 2014-04-18 21:36:31.016029014 +0800
Modify: 2014-04-18 21:36:31.040032065 +0800
Change: 2014-04-18 21:36:31.040032065 +0800
Birth: -
With no_root_squash, (root, remote ext4),
should not change the sgid/suid, but got
File: ‘test’
Size: 0 Blocks: 0 IO Block: 262144 regular
empty file
Device: 24h/36d Inode: 786439 Links: 1
Access: (4777/-rwsrwxrwx) Uid: ( 1000/ test) Gid: ( 1000/ test)
Context: system_u:object_r:nfs_t:s0
Access: 2014-04-18 21:45:32.155805097 +0800
Modify: 2014-04-18 21:45:32.155805097 +0800
Change: 2014-04-18 21:45:32.168806749 +0800
Birth: -
File: ‘test’
Size: 5 Blocks: 8 IO Block: 262144 regular file
Device: 24h/36d Inode: 786439 Links: 1
Access: (0777/-rwxrwxrwx) Uid: ( 1000/ test) Gid: ( 1000/ test)
Context: system_u:object_r:nfs_t:s0
Access: 2014-04-18 21:45:32.155805097 +0800
Modify: 2014-04-18 21:45:32.184808783 +0800
Change: 2014-04-18 21:45:32.184808783 +0800
Birth: -
thanks,
Kinglong Mee
I don't see an owner check in should_remove_suid.
And I think that an nfsd thread will always have CAP_FSETID set (see
cap_raise_nfsd_set and the definition of CAP_NFSD_SET), so that
should_remove_suid() will always return 0.
--b.
Signed-off-by: Kinglong Mee <kinglongmee@xxxxxxxxx>
---
fs/nfsd/vfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 16f0673..19c0931 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -943,7 +943,7 @@ nfsd_vfs_write(struct svc_rqst *rqstp, struct svc_fh
*fhp, struct file *file,
fsnotify_modify(file);
/* clear setuid/setgid flag after write */
- if (inode->i_mode & (S_ISUID | S_ISGID))
+ if (should_remove_suid(dentry))
kill_suid(dentry);
if (stable) {
--
1.9.0
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
