Skip to content

Commit 76a0e79

Browse files
scottmayhewpcmoore
authored andcommitted
selinux,smack: don't bypass permissions check in inode_setsecctx hook
Marek Gresko reports that the root user on an NFS client is able to change the security labels on files on an NFS filesystem that is exported with root squashing enabled. The end of the kerneldoc comment for __vfs_setxattr_noperm() states: * This function requires the caller to lock the inode's i_mutex before it * is executed. It also assumes that the caller will make the appropriate * permission checks. nfsd_setattr() does do permissions checking via fh_verify() and nfsd_permission(), but those don't do all the same permissions checks that are done by security_inode_setxattr() and its related LSM hooks do. Since nfsd_setattr() is the only consumer of security_inode_setsecctx(), simplest solution appears to be to replace the call to __vfs_setxattr_noperm() with a call to __vfs_setxattr_locked(). This fixes the above issue and has the added benefit of causing nfsd to recall conflicting delegations on a file when a client tries to change its security label. Cc: [email protected] Reported-by: Marek Gresko <[email protected]> Link: https://bugzilla.kernel.org/show_bug.cgi?id=218809 Signed-off-by: Scott Mayhew <[email protected]> Tested-by: Stephen Smalley <[email protected]> Reviewed-by: Stephen Smalley <[email protected]> Reviewed-by: Chuck Lever <[email protected]> Reviewed-by: Jeff Layton <[email protected]> Acked-by: Casey Schaufler <[email protected]> Signed-off-by: Paul Moore <[email protected]>
1 parent 8400291 commit 76a0e79

File tree

2 files changed

+4
-4
lines changed

2 files changed

+4
-4
lines changed

security/selinux/hooks.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6650,8 +6650,8 @@ static int selinux_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen
66506650
*/
66516651
static int selinux_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen)
66526652
{
6653-
return __vfs_setxattr_noperm(&nop_mnt_idmap, dentry, XATTR_NAME_SELINUX,
6654-
ctx, ctxlen, 0);
6653+
return __vfs_setxattr_locked(&nop_mnt_idmap, dentry, XATTR_NAME_SELINUX,
6654+
ctx, ctxlen, 0, NULL);
66556655
}
66566656

66576657
static int selinux_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)

security/smack/smack_lsm.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4880,8 +4880,8 @@ static int smack_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
48804880

48814881
static int smack_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen)
48824882
{
4883-
return __vfs_setxattr_noperm(&nop_mnt_idmap, dentry, XATTR_NAME_SMACK,
4884-
ctx, ctxlen, 0);
4883+
return __vfs_setxattr_locked(&nop_mnt_idmap, dentry, XATTR_NAME_SMACK,
4884+
ctx, ctxlen, 0, NULL);
48854885
}
48864886

48874887
static int smack_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)

0 commit comments

Comments
 (0)