From 0fb0a762bd3f5f0fb4a2e026accf01bf9d4b7e1c Mon Sep 17 00:00:00 2001 From: Scott Mayhew Date: Wed, 25 Sep 2024 19:01:01 +0800 Subject: [PATCH] selinux,smack: don't bypass permissions check in inode_setsecctx hook mainline inclusion from mainline-v6.11-rc6 commit 76a0e79bc84f466999fa501fce5bf7a07641b8a7 category: bugfix bugzilla: https://gitee.com/src-openeuler/kernel/issues/IAR4JE CVE: CVE-2024-46695 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=76a0e79bc84f466999fa501fce5bf7a07641b8a7 -------------------------------- 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: stable@kernel.org Reported-by: Marek Gresko Link: https://bugzilla.kernel.org/show_bug.cgi?id=218809 Signed-off-by: Scott Mayhew Tested-by: Stephen Smalley Reviewed-by: Stephen Smalley Reviewed-by: Chuck Lever Reviewed-by: Jeff Layton Acked-by: Casey Schaufler Signed-off-by: Paul Moore Conflicts: security/selinux/hooks.c security/smack/smack_lsm.c [Due to the lack of mnt_idmap feature in 5.10, which brings a new input argument for both __vfs_setxattr_locked/noperm().] Signed-off-by: GONG Ruiqi (cherry picked from commit a17d856f520a7af99a767eb99e27e4de7598a017) --- security/selinux/hooks.c | 2 +- security/smack/smack_lsm.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index ee37ce2e2619..1376cd9311fc 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6537,7 +6537,7 @@ static int selinux_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen */ static int selinux_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen) { - return __vfs_setxattr_noperm(dentry, XATTR_NAME_SELINUX, ctx, ctxlen, 0); + return __vfs_setxattr_locked(dentry, XATTR_NAME_SELINUX, ctx, ctxlen, 0, NULL); } static int selinux_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen) diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 2961f84a54df..fa4ca557c7dd 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -4605,7 +4605,7 @@ static int smack_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen) static int smack_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen) { - return __vfs_setxattr_noperm(dentry, XATTR_NAME_SMACK, ctx, ctxlen, 0); + return __vfs_setxattr_locked(dentry, XATTR_NAME_SMACK, ctx, ctxlen, 0, NULL); } static int smack_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen) -- Gitee