Re: [PATCH 1/4] Cache xattr security drop check for write v2

[Steven Whitehouse <swhiteho@redhat.com>]

Hi,

On Sat, 2011-05-28 at 08:25 -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Some recent benchmarking on btrfs showed that a major scaling bottleneck
> on large systems on btrfs is currently the xattr lookup on every write.
> 
> Why xattr lookup on every write I hear you ask?
> 
> write wants to drop suid and security related xattrs that could set o
> capabilities for executables.  To do that it currently looks up
> security.capability on EVERY write (even for non executables) to decide
> whether to drop it or not.
> 
It sounds like a good idea, but cluster filesystems will need to clear
the flag when they update their in-core inodes. Without that we could
have:

 Node A looks up inode and sets S_NOSEC since its not suid
 Node B does chmod +s on the inode
 Node A now has S_NOSEC set, but inode is suid, so writes don't clear
suid

For GFS2 it should simply be a case of adjusting gfs2_set_inode_flags()
to update S_NOSEC appropriately, something like this (untested):

diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index a9f5cbe..3d856e4 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -174,7 +174,9 @@ void gfs2_set_inode_flags(struct inode *inode)
 	struct gfs2_inode *ip = GFS2_I(inode);
 	unsigned int flags = inode->i_flags;
 
-	flags &= ~(S_SYNC|S_APPEND|S_IMMUTABLE|S_NOATIME|S_DIRSYNC);
+	flags &= ~(S_SYNC|S_APPEND|S_IMMUTABLE|S_NOATIME|S_DIRSYNC|S_NOSEC);
+	if (!is_sxid(inode->i_mode))
+		flags |= S_NOSEC;
 	if (ip->i_diskflags & GFS2_DIF_IMMUTABLE)
 		flags |= S_IMMUTABLE;
 	if (ip->i_diskflags & GFS2_DIF_APPENDONLY)


Note that this also serves the dual purpose of setting the flag for
newly created inodes as well, as per the patches for the other
filesystems,

Steve.