Re: Calls to vfs_setlease() from NFSD code cause unnecessary CAP_LEASE security checks
From: Ondrej Mosnacek <omosnace@redhat.com>
Date: 2024-02-02 16:31:21
Also in:
linux-fsdevel, linux-nfs, selinux
On Fri, Feb 2, 2024 at 5:08 PM Jeff Layton [off-list ref] wrote:
On Fri, 2024-02-02 at 16:31 +0100, Ondrej Mosnacek wrote:quoted
Hello, In [1] a user reports seeing SELinux denials from NFSD when it writes into /proc/fs/nfsd/threads with the following kernel backtrace: => trace_event_raw_event_selinux_audited => avc_audit_post_callback => common_lsm_audit => slow_avc_audit => cred_has_capability.isra.0 => security_capable => capable => generic_setlease => destroy_unhashed_deleg => __destroy_client => nfs4_state_shutdown_net => nfsd_shutdown_net => nfsd_last_thread => nfsd_svc => write_threads => nfsctl_transaction_write => vfs_write => ksys_write => do_syscall_64 => entry_SYSCALL_64_after_hwframe It seems to me that the security checks in generic_setlease() should be skipped (at least) when called through this codepath, since the userspace process merely writes into /proc/fs/nfsd/threads and it's just the kernel's internal code that releases the lease as a side effect. For example, for vfs_write() there is kernel_write(), which provides a no-security-check equivalent. Should there be something similar for vfs_setlease() that could be utilized for this purpose? [1] https://bugzilla.redhat.com/show_bug.cgi?id=2248830Thanks for the bug report! Am I correct that we only want to do this check when someone from userland tries to set a lease via fcntl? The rest of the callers are all in-kernel callers and I don't think we need to check for any of them. It may be simpler to just push this check into the appropriate callers of generic_setlease instead. Hmm now that I look too...it looks like we aren't checking CAP_LEASE on filesystems that have their own ->setlease operation. I'll have a look at that soon too.
I did briefly check this while analyzing the issue and all of the setlease fops implementations seemed to be either simple_nosetlease() or some wrappers around generic_setlease(), which should both be OK. But it can't hurt to double-check :) -- Ondrej Mosnacek Senior Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.