[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20251119-dir-deleg-ro-v8-3-81b6cf5485c6@kernel.org>
Date: Wed, 19 Nov 2025 08:42:20 -0500
From: Jeff Layton <jlayton@...nel.org>
To: Alexander Viro <viro@...iv.linux.org.uk>,
Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
Jeff Layton <jlayton@...nel.org>, Chuck Lever <chuck.lever@...cle.com>,
Alexander Aring <alex.aring@...il.com>, NeilBrown <neil@...wn.name>,
Olga Kornievskaia <okorniev@...hat.com>, Dai Ngo <Dai.Ngo@...cle.com>,
Tom Talpey <tom@...pey.com>
Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-nfs@...r.kernel.org, Stephen Rothwell <sfr@...b.auug.org.au>
Subject: [PATCH v8 3/3] 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.
Signed-off-by: Jeff Layton <jlayton@...nel.org>
---
fs/locks.c | 90 ++++++++++++++++++++++--------------------------
fs/nfsd/nfs4layouts.c | 11 ++++--
fs/nfsd/nfs4state.c | 7 ++++
include/linux/filelock.h | 1 +
4 files changed, 59 insertions(+), 50 deletions(-)
diff --git a/fs/locks.c b/fs/locks.c
index d4e6af6ac625204b337e94fd1e4f6df2eee5cf50..62fbfce0407b77423e1591290cf57c4e2c5faeb4 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -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,
};
/*
@@ -1762,52 +1802,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)
{
@@ -1844,7 +1838,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;
@@ -1901,7 +1895,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;
diff --git a/fs/nfsd/nfs4layouts.c b/fs/nfsd/nfs4layouts.c
index 683bd1130afe298f9df774684192c89f68102b72..ca7ec7a022bd5c12fad60ff9e51145d9cca55527 100644
--- a/fs/nfsd/nfs4layouts.c
+++ b/fs/nfsd/nfs4layouts.c
@@ -764,9 +764,16 @@ nfsd4_layout_lm_change(struct file_lease *onlist, int arg,
return lease_modify(onlist, arg, dispose);
}
+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_break = nfsd4_layout_lm_break,
+ .lm_change = nfsd4_layout_lm_change,
+ .lm_open_conflict = nfsd4_layout_lm_open_conflict,
};
int
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 8f8c9385101e15b64883eabec71775f26b14f890..669fabb095407e61525e5b71268cf1f06fc09877 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5543,10 +5543,17 @@ nfsd_change_deleg_cb(struct file_lease *onlist, int arg,
return -EAGAIN;
}
+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)
diff --git a/include/linux/filelock.h b/include/linux/filelock.h
index 54b824c05299261e6bd6acc4175cb277ea35b35d..2f5e5588ee0733c200103801d0d2ba19bebbf9af 100644
--- a/include/linux/filelock.h
+++ b/include/linux/filelock.h
@@ -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 {
--
2.51.1
Powered by blists - more mailing lists