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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 31 May 2011 14:51:36 +0100
From:	Steven Whitehouse <swhiteho@...hat.com>
To:	Andi Kleen <andi@...stfloor.org>
Cc:	viro@...iv.linux.org.uk, linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, Andi Kleen <ak@...ux.intel.com>,
	chris.mason@...cle.com, josef@...hat.com, agruen@...bit.com,
	"Serge E. Hallyn" <serue@...ibm.com>
Subject: Re: [PATCH 1/4] Cache xattr security drop check for write v2

Hi,

On Sat, 2011-05-28 at 08:25 -0700, Andi Kleen wrote:
> From: Andi Kleen <ak@...ux.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.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/