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

Steven Whitehouse
2011-05-31T13:50:14Z

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.