Calls to vfs_setlease() from NFSD code cause unnecessary CAP_LEASE security checks

[Jeff Layton]

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.

Thanks,
-- 
Jeff Layton <jlayton at kernel.org>