Commit 7b800101 authored by Jeff Layton's avatar Jeff Layton Committed by Christian Brauner
Browse files

filelock: don't do security checks on nfsd setlease calls 



Zdenek reported seeing some AVC denials due to nfsd trying to set
delegations:

    type=AVC msg=audit(09.11.2023 09:03:46.411:496) : avc:  denied  { lease } for  pid=5127 comm=rpc.nfsd capability=lease  scontext=system_u:system_r:nfsd_t:s0 tcontext=system_u:system_r:nfsd_t:s0 tclass=capability permissive=0

When setting delegations on behalf of nfsd, we don't want to do all of
the normal capabilty and LSM checks. nfsd is a kernel thread and runs
with CAP_LEASE set, so the uid checks end up being a no-op in most cases
anyway.

Some nfsd functions can end up running in normal process context when
tearing down the server. At that point, the CAP_LEASE check can fail and
cause the client to not tear down delegations when expected.

Also, the way the per-fs ->setlease handlers work today is a little
convoluted. The non-trivial ones are wrappers around generic_setlease,
so when they fail due to permission problems they usually they end up
doing a little extra work only to determine that they can't set the
lease anyway. It would be more efficient to do those checks earlier.

Transplant the permission checking from generic_setlease to
vfs_setlease, which will make the permission checking happen earlier on
filesystems that have a ->setlease operation. Add a new kernel_setlease
function that bypasses these checks, and switch nfsd to use that instead
of vfs_setlease.

There is one behavioral change here: prior this patch the
setlease_notifier would fire even if the lease attempt was going to fail
the security checks later. With this change, it doesn't fire until the
caller has passed them. I think this is a desirable change overall. nfsd
is the only user of the setlease_notifier and it doesn't benefit from
being notified about failed attempts.

Cc: Ondrej Mosnáček <omosnacek@gmail.com>
Reported-by: default avatarZdenek Pytela <zpytela@redhat.com>
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2248830


Signed-off-by: default avatarJeff Layton <jlayton@kernel.org>
Link: https://lore.kernel.org/r/20240205-bz2248830-v1-1-d0ec0daecba1@kernel.org


Acked-by: default avatarTom Talpey <tom@talpey.com>
Reviewed-by: default avatarNeilBrown <neilb@suse.de>
Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
parent c4b3ffb5

fs/locks.c

+25 −18
Original line number Diff line number Diff line
@@ -1925,18 +1925,6 @@ static int generic_delete_lease(struct file *filp, void *owner) 
int generic_setlease(struct file *filp, int arg, struct file_lease **flp,

			void **priv)

{

	struct inode *inode = file_inode(filp);

	vfsuid_t vfsuid = i_uid_into_vfsuid(file_mnt_idmap(filp), inode);

	int error;



	if ((!vfsuid_eq_kuid(vfsuid, current_fsuid())) && !capable(CAP_LEASE))

		return -EACCES;

	if (!S_ISREG(inode->i_mode))

		return -EINVAL;

	error = security_file_lock(filp, arg);

	if (error)

		return error;



	switch (arg) {

	case F_UNLCK:

		return generic_delete_lease(filp, *priv);
@@ -1987,6 +1975,19 @@ void lease_unregister_notifier(struct notifier_block *nb) 
}

EXPORT_SYMBOL_GPL(lease_unregister_notifier);





int

kernel_setlease(struct file *filp, int arg, struct file_lease **lease, void **priv)

{

	if (lease)

		setlease_notifier(arg, *lease);

	if (filp->f_op->setlease)

		return filp->f_op->setlease(filp, arg, lease, priv);

	else

		return generic_setlease(filp, arg, lease, priv);

}

EXPORT_SYMBOL_GPL(kernel_setlease);



/**

 * vfs_setlease        -       sets a lease on an open file

 * @filp:	file pointer
@@ -2007,12 +2008,18 @@ EXPORT_SYMBOL_GPL(lease_unregister_notifier); 
int

vfs_setlease(struct file *filp, int arg, struct file_lease **lease, void **priv)

{

	if (lease)

		setlease_notifier(arg, *lease);

	if (filp->f_op->setlease)

		return filp->f_op->setlease(filp, arg, lease, priv);

	else

		return generic_setlease(filp, arg, lease, priv);

	struct inode *inode = file_inode(filp);

	vfsuid_t vfsuid = i_uid_into_vfsuid(file_mnt_idmap(filp), inode);

	int error;



	if ((!vfsuid_eq_kuid(vfsuid, current_fsuid())) && !capable(CAP_LEASE))

		return -EACCES;

	if (!S_ISREG(inode->i_mode))

		return -EINVAL;

	error = security_file_lock(filp, arg);

	if (error)

		return error;

	return kernel_setlease(filp, arg, lease, priv);

}

EXPORT_SYMBOL_GPL(vfs_setlease);

fs/nfsd/nfs4layouts.c

+2 −3
Original line number Diff line number Diff line
@@ -170,7 +170,7 @@ nfsd4_free_layout_stateid(struct nfs4_stid *stid) 
	spin_unlock(&fp->fi_lock);



	if (!nfsd4_layout_ops[ls->ls_layout_type]->disable_recalls)

		vfs_setlease(ls->ls_file->nf_file, F_UNLCK, NULL, (void **)&ls);

		kernel_setlease(ls->ls_file->nf_file, F_UNLCK, NULL, (void **)&ls);

	nfsd_file_put(ls->ls_file);



	if (ls->ls_recalled)
@@ -199,8 +199,7 @@ nfsd4_layout_setlease(struct nfs4_layout_stateid *ls) 
	fl->c.flc_pid = current->tgid;

	fl->c.flc_file = ls->ls_file->nf_file;



	status = vfs_setlease(fl->c.flc_file, fl->c.flc_type, &fl,

			      NULL);

	status = kernel_setlease(fl->c.flc_file, fl->c.flc_type, &fl, NULL);

	if (status) {

		locks_free_lease(fl);

		return status;

fs/nfsd/nfs4state.c

+4 −4
Original line number Diff line number Diff line
@@ -1249,7 +1249,7 @@ static void nfs4_unlock_deleg_lease(struct nfs4_delegation *dp) 


	WARN_ON_ONCE(!fp->fi_delegees);



	vfs_setlease(nf->nf_file, F_UNLCK, NULL, (void **)&dp);

	kernel_setlease(nf->nf_file, F_UNLCK, NULL, (void **)&dp);

	put_deleg_file(fp);

}
@@ -5532,7 +5532,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, 
	if (!fl)

		goto out_clnt_odstate;



	status = vfs_setlease(fp->fi_deleg_file->nf_file,

	status = kernel_setlease(fp->fi_deleg_file->nf_file,

				      fl->c.flc_type, &fl, NULL);

	if (fl)

		locks_free_lease(fl);
@@ -5571,7 +5571,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp, 


	return dp;

out_unlock:

	vfs_setlease(fp->fi_deleg_file->nf_file, F_UNLCK, NULL, (void **)&dp);

	kernel_setlease(fp->fi_deleg_file->nf_file, F_UNLCK, NULL, (void **)&dp);

out_clnt_odstate:

	put_clnt_odstate(dp->dl_clnt_odstate);

	nfs4_put_stid(&dp->dl_stid);

include/linux/filelock.h

+7 −0
Original line number Diff line number Diff line
@@ -208,6 +208,7 @@ struct file_lease *locks_alloc_lease(void); 
int __break_lease(struct inode *inode, unsigned int flags, unsigned int type);

void lease_get_mtime(struct inode *, struct timespec64 *time);

int generic_setlease(struct file *, int, struct file_lease **, void **priv);

int kernel_setlease(struct file *, int, struct file_lease **, void **);

int vfs_setlease(struct file *, int, struct file_lease **, void **);

int lease_modify(struct file_lease *, int, struct list_head *);
@@ -378,6 +379,12 @@ static inline int generic_setlease(struct file *filp, int arg, 
	return -EINVAL;

}



static inline int kernel_setlease(struct file *filp, int arg,

			       struct file_lease **lease, void **priv)

{

	return -EINVAL;

}



static inline int vfs_setlease(struct file *filp, int arg,

			       struct file_lease **lease, void **priv)

{