On Fri, Feb 2, 2024 at 5:31 PM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > > 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 :) To close the loop here - there is now a fix from Jeff in linux-next: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=7b8001013d720c232ad9ae7aae0ef0e7c281c6d4 Thank you, Jeff, for taking care of it! -- Ondrej Mosnacek Senior Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.
