Unverified Commit 24835a96 authored by Christian Brauner's avatar Christian Brauner
Browse files

Merge patch series "filelock: fix conflict detection with userland file delegations" 

Jeff Layton <jlayton@kernel.org> says:

This patchset fixes the way that conflicts are detected when userland
requests file delegations. The problem is due to a hack that was added
long ago which worked up until userland could request a file delegation.

This fixes the bug and makes things a bit less hacky. Please consider
for v6.19.

* patches from https://patch.msgid.link/20251204-dir-deleg-ro-v2-0-22d37f92ce2c@kernel.org:
  filelock: allow lease_managers to dictate what qualifies as a conflict
  filelock: add lease_dispose_list() helper

Link: https://patch.msgid.link/20251204-dir-deleg-ro-v2-0-22d37f92ce2c@kernel.org


Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
parents ed61378b 12965a19

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

+61 −58
Original line number Diff line number Diff line
@@ -369,13 +369,22 @@ locks_dispose_list(struct list_head *dispose) 
	while (!list_empty(dispose)) {

		flc = list_first_entry(dispose, struct file_lock_core, flc_list);

		list_del_init(&flc->flc_list);

		if (flc->flc_flags & (FL_LEASE|FL_DELEG|FL_LAYOUT))

			locks_free_lease(file_lease(flc));

		else

		locks_free_lock(file_lock(flc));

	}

}



static void

lease_dispose_list(struct list_head *dispose)

{

	struct file_lock_core *flc;



	while (!list_empty(dispose)) {

		flc = list_first_entry(dispose, struct file_lock_core, flc_list);

		list_del_init(&flc->flc_list);

		locks_free_lease(file_lease(flc));

	}

}



void locks_init_lock(struct file_lock *fl)

{

	memset(fl, 0, sizeof(struct file_lock));
@@ -576,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,

};



/*
@@ -1620,7 +1669,7 @@ int __break_lease(struct inode *inode, unsigned int flags) 
	spin_unlock(&ctx->flc_lock);

	percpu_up_read(&file_rwsem);



	locks_dispose_list(&dispose);

	lease_dispose_list(&dispose);

	error = wait_event_interruptible_timeout(new_fl->c.flc_wait,

						 list_empty(&new_fl->c.flc_blocked_member),

						 break_time);
@@ -1643,7 +1692,7 @@ int __break_lease(struct inode *inode, unsigned int flags) 
out:

	spin_unlock(&ctx->flc_lock);

	percpu_up_read(&file_rwsem);

	locks_dispose_list(&dispose);

	lease_dispose_list(&dispose);

free_lock:

	locks_free_lease(new_fl);

	return error;
@@ -1727,7 +1776,7 @@ static int __fcntl_getlease(struct file *filp, unsigned int flavor) 
		spin_unlock(&ctx->flc_lock);

		percpu_up_read(&file_rwsem);



		locks_dispose_list(&dispose);

		lease_dispose_list(&dispose);

	}

	return type;

}
@@ -1745,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)

{
@@ -1827,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;
@@ -1884,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;
@@ -1896,7 +1899,7 @@ generic_add_lease(struct file *filp, int arg, struct file_lease **flp, void **pr 
out:

	spin_unlock(&ctx->flc_lock);

	percpu_up_read(&file_rwsem);

	locks_dispose_list(&dispose);

	lease_dispose_list(&dispose);

	if (is_deleg)

		inode_unlock(inode);

	if (!error && !my_fl)
@@ -1932,7 +1935,7 @@ static int generic_delete_lease(struct file *filp, void *owner) 
		error = fl->fl_lmops->lm_change(victim, F_UNLCK, &dispose);

	spin_unlock(&ctx->flc_lock);

	percpu_up_read(&file_rwsem);

	locks_dispose_list(&dispose);

	lease_dispose_list(&dispose);

	return error;

}
@@ -2727,7 +2730,7 @@ locks_remove_lease(struct file *filp, struct file_lock_context *ctx) 
	spin_unlock(&ctx->flc_lock);

	percpu_up_read(&file_rwsem);



	locks_dispose_list(&dispose);

	lease_dispose_list(&dispose);

}



/*

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 {