On Wed, Aug 28, 2024 at 5:29 PM Casey Schaufler <casey@xxxxxxxxxxxxxxxx> wrote: > On 8/28/2024 2:05 PM, Paul Moore wrote: > > On Wed, Aug 28, 2024 at 3:51 PM Scott Mayhew <smayhew@xxxxxxxxxx> wrote: > >> 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. > >> > >> Reported-by: Marek Gresko <marek.gresko@xxxxxxxxxxxxxx> > >> Link: https://bugzilla.kernel.org/show_bug.cgi?id=218809 > >> Signed-off-by: Scott Mayhew <smayhew@xxxxxxxxxx> > >> --- > >> security/selinux/hooks.c | 4 ++-- > >> security/smack/smack_lsm.c | 4 ++-- > >> 2 files changed, 4 insertions(+), 4 deletions(-) > > Thanks Scott, this looks good to me, but since it touches Smack too > > I'd also like to get Casey's ACK on this patch; > > Testing labeled NFS has always been a challenge for the somewhat > limited resources available to the Smack project. I'll Ack the patch, > but won't claim to have tested it. Understood, thanks for the quick reply. -- paul-moore.com
