Unverified Commit 12965a19 authored by Jeff Layton's avatar Jeff Layton Committed by Christian Brauner
Browse files

filelock: allow lease_managers to dictate what qualifies as a conflict 



Requesting a delegation on a file from the userland fcntl() interface
currently succeeds when there are conflicting opens present.

This is because the lease handling code ignores conflicting opens for
FL_LAYOUT and FL_DELEG leases. This was a hack put in place long ago,
because nfsd already checks for conflicts in its own way. The kernel
needs to perform this check for userland delegations the same way it is
done for leases, however.

Make this dependent on the lease_manager by adding a new
->lm_open_conflict() lease_manager operation and have
generic_add_lease() call that instead of check_conflicting_open().
Morph check_conflicting_open() into a ->lm_open_conflict() op that is
only called for userland leases/delegations. Set the
->lm_open_conflict() operations for nfsd to trivial functions that
always return 0.

Reviewed-by: default avatarChuck Lever <chuck.lever@oracle.com>
Signed-off-by: default avatarJeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20251204-dir-deleg-ro-v2-2-22d37f92ce2c@kernel.org


Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
parent 392e317a

Documentation/filesystems/locking.rst

+1 −0
Original line number Diff line number Diff line
@@ -416,6 +416,7 @@ lm_change yes no no 
lm_breaker_owns_lease:	yes     	no			no

lm_lock_expirable	yes		no			no

lm_expire_lock		no		no			yes

lm_open_conflict	yes		no			no

======================	=============	=================	=========



buffer_head

fs/locks.c

+42 −48
Original line number Diff line number Diff line
@@ -585,10 +585,50 @@ lease_setup(struct file_lease *fl, void **priv) 
	__f_setown(filp, task_pid(current), PIDTYPE_TGID, 0);

}



/**

 * lease_open_conflict - see if the given file points to an inode that has

 *			 an existing open that would conflict with the

 *			 desired lease.

 * @filp:	file to check

 * @arg:	type of lease that we're trying to acquire

 *

 * Check to see if there's an existing open fd on this file that would

 * conflict with the lease we're trying to set.

 */

static int

lease_open_conflict(struct file *filp, const int arg)

{

	struct inode *inode = file_inode(filp);

	int self_wcount = 0, self_rcount = 0;



	if (arg == F_RDLCK)

		return inode_is_open_for_write(inode) ? -EAGAIN : 0;

	else if (arg != F_WRLCK)

		return 0;



	/*

	 * Make sure that only read/write count is from lease requestor.

	 * Note that this will result in denying write leases when i_writecount

	 * is negative, which is what we want.  (We shouldn't grant write leases

	 * on files open for execution.)

	 */

	if (filp->f_mode & FMODE_WRITE)

		self_wcount = 1;

	else if (filp->f_mode & FMODE_READ)

		self_rcount = 1;



	if (atomic_read(&inode->i_writecount) != self_wcount ||

	    atomic_read(&inode->i_readcount) != self_rcount)

		return -EAGAIN;



	return 0;

}



static const struct lease_manager_operations lease_manager_ops = {

	.lm_break = lease_break_callback,

	.lm_change = lease_modify,

	.lm_setup = lease_setup,

	.lm_open_conflict = lease_open_conflict,

};



/*
@@ -1754,52 +1794,6 @@ int fcntl_getdeleg(struct file *filp, struct delegation *deleg) 
	return 0;

}



/**

 * check_conflicting_open - see if the given file points to an inode that has

 *			    an existing open that would conflict with the

 *			    desired lease.

 * @filp:	file to check

 * @arg:	type of lease that we're trying to acquire

 * @flags:	current lock flags

 *

 * Check to see if there's an existing open fd on this file that would

 * conflict with the lease we're trying to set.

 */

static int

check_conflicting_open(struct file *filp, const int arg, int flags)

{

	struct inode *inode = file_inode(filp);

	int self_wcount = 0, self_rcount = 0;



	if (flags & FL_LAYOUT)

		return 0;

	if (flags & FL_DELEG)

		/* We leave these checks to the caller */

		return 0;



	if (arg == F_RDLCK)

		return inode_is_open_for_write(inode) ? -EAGAIN : 0;

	else if (arg != F_WRLCK)

		return 0;



	/*

	 * Make sure that only read/write count is from lease requestor.

	 * Note that this will result in denying write leases when i_writecount

	 * is negative, which is what we want.  (We shouldn't grant write leases

	 * on files open for execution.)

	 */

	if (filp->f_mode & FMODE_WRITE)

		self_wcount = 1;

	else if (filp->f_mode & FMODE_READ)

		self_rcount = 1;



	if (atomic_read(&inode->i_writecount) != self_wcount ||

	    atomic_read(&inode->i_readcount) != self_rcount)

		return -EAGAIN;



	return 0;

}



static int

generic_add_lease(struct file *filp, int arg, struct file_lease **flp, void **priv)

{
@@ -1836,7 +1830,7 @@ generic_add_lease(struct file *filp, int arg, struct file_lease **flp, void **pr 
	percpu_down_read(&file_rwsem);

	spin_lock(&ctx->flc_lock);

	time_out_leases(inode, &dispose);

	error = check_conflicting_open(filp, arg, lease->c.flc_flags);

	error = lease->fl_lmops->lm_open_conflict(filp, arg);

	if (error)

		goto out;
@@ -1893,7 +1887,7 @@ generic_add_lease(struct file *filp, int arg, struct file_lease **flp, void **pr 
	 * precedes these checks.

	 */

	smp_mb();

	error = check_conflicting_open(filp, arg, lease->c.flc_flags);

	error = lease->fl_lmops->lm_open_conflict(filp, arg);

	if (error) {

		locks_unlink_lock_ctx(&lease->c);

		goto out;

fs/nfsd/nfs4layouts.c

+21 −2
Original line number Diff line number Diff line
@@ -764,9 +764,28 @@ nfsd4_layout_lm_change(struct file_lease *onlist, int arg, 
	return lease_modify(onlist, arg, dispose);

}



/**

 *  nfsd4_layout_lm_open_conflict - see if the given file points to an inode that has

 *				    an existing open that would conflict with the

 *				    desired lease.

 * @filp:	file to check

 * @arg:	type of lease that we're trying to acquire

 *

 * The kernel will call into this operation to determine whether there

 * are conflicting opens that may prevent the layout from being granted.

 * For nfsd, that check is done at a higher level, so this trivially

 * returns 0.

 */

static int

nfsd4_layout_lm_open_conflict(struct file *filp, int arg)

{

	return 0;

}



static const struct lease_manager_operations nfsd4_layouts_lm_ops = {

	.lm_break		= nfsd4_layout_lm_break,

	.lm_change		= nfsd4_layout_lm_change,

	.lm_open_conflict	= nfsd4_layout_lm_open_conflict,

};



int

fs/nfsd/nfs4state.c

+19 −0
Original line number Diff line number Diff line
@@ -5552,10 +5552,29 @@ nfsd_change_deleg_cb(struct file_lease *onlist, int arg, 
		return -EAGAIN;

}



/**

 *  nfsd4_deleg_lm_open_conflict - see if the given file points to an inode that has

 *				   an existing open that would conflict with the

 *				   desired lease.

 * @filp:	file to check

 * @arg:	type of lease that we're trying to acquire

 *

 * The kernel will call into this operation to determine whether there

 * are conflicting opens that may prevent the deleg from being granted.

 * For nfsd, that check is done at a higher level, so this trivially

 * returns 0.

 */

static int

nfsd4_deleg_lm_open_conflict(struct file *filp, int arg)

{

	return 0;

}



static const struct lease_manager_operations nfsd_lease_mng_ops = {

	.lm_breaker_owns_lease = nfsd_breaker_owns_lease,

	.lm_break = nfsd_break_deleg_cb,

	.lm_change = nfsd_change_deleg_cb,

	.lm_open_conflict = nfsd4_deleg_lm_open_conflict,

};



static __be32 nfsd4_check_seqid(struct nfsd4_compound_state *cstate, struct nfs4_stateowner *so, u32 seqid)

include/linux/filelock.h

+1 −0
Original line number Diff line number Diff line
@@ -49,6 +49,7 @@ struct lease_manager_operations { 
	int (*lm_change)(struct file_lease *, int, struct list_head *);

	void (*lm_setup)(struct file_lease *, void **);

	bool (*lm_breaker_owns_lease)(struct file_lease *);

	int (*lm_open_conflict)(struct file *, int);

};



struct lock_manager {