On Fri, Feb 2, 2024 at 5:08 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote: > > On Fri, 2024-02-02 at 16:31 +0100, Ondrej Mosnacek wrote: > > 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=2248830 > > > > Thanks 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.
