Skip to content

Commit 12965a1

Browse files
jtlaytonbrauner
authored andcommitted
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: Chuck Lever <chuck.lever@oracle.com> Signed-off-by: Jeff Layton <jlayton@kernel.org> Link: https://patch.msgid.link/20251204-dir-deleg-ro-v2-2-22d37f92ce2c@kernel.org Signed-off-by: Christian Brauner <brauner@kernel.org>
1 parent 392e317 commit 12965a1

File tree

5 files changed

+84
-50
lines changed

5 files changed

+84
-50
lines changed

Documentation/filesystems/locking.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,7 @@ lm_change yes no no
416416
lm_breaker_owns_lease: yes no no
417417
lm_lock_expirable yes no no
418418
lm_expire_lock no no yes
419+
lm_open_conflict yes no no
419420
====================== ============= ================= =========
420421

421422
buffer_head

fs/locks.c

Lines changed: 42 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -585,10 +585,50 @@ lease_setup(struct file_lease *fl, void **priv)
585585
__f_setown(filp, task_pid(current), PIDTYPE_TGID, 0);
586586
}
587587

588+
/**
589+
* lease_open_conflict - see if the given file points to an inode that has
590+
* an existing open that would conflict with the
591+
* desired lease.
592+
* @filp: file to check
593+
* @arg: type of lease that we're trying to acquire
594+
*
595+
* Check to see if there's an existing open fd on this file that would
596+
* conflict with the lease we're trying to set.
597+
*/
598+
static int
599+
lease_open_conflict(struct file *filp, const int arg)
600+
{
601+
struct inode *inode = file_inode(filp);
602+
int self_wcount = 0, self_rcount = 0;
603+
604+
if (arg == F_RDLCK)
605+
return inode_is_open_for_write(inode) ? -EAGAIN : 0;
606+
else if (arg != F_WRLCK)
607+
return 0;
608+
609+
/*
610+
* Make sure that only read/write count is from lease requestor.
611+
* Note that this will result in denying write leases when i_writecount
612+
* is negative, which is what we want. (We shouldn't grant write leases
613+
* on files open for execution.)
614+
*/
615+
if (filp->f_mode & FMODE_WRITE)
616+
self_wcount = 1;
617+
else if (filp->f_mode & FMODE_READ)
618+
self_rcount = 1;
619+
620+
if (atomic_read(&inode->i_writecount) != self_wcount ||
621+
atomic_read(&inode->i_readcount) != self_rcount)
622+
return -EAGAIN;
623+
624+
return 0;
625+
}
626+
588627
static const struct lease_manager_operations lease_manager_ops = {
589628
.lm_break = lease_break_callback,
590629
.lm_change = lease_modify,
591630
.lm_setup = lease_setup,
631+
.lm_open_conflict = lease_open_conflict,
592632
};
593633

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

1757-
/**
1758-
* check_conflicting_open - see if the given file points to an inode that has
1759-
* an existing open that would conflict with the
1760-
* desired lease.
1761-
* @filp: file to check
1762-
* @arg: type of lease that we're trying to acquire
1763-
* @flags: current lock flags
1764-
*
1765-
* Check to see if there's an existing open fd on this file that would
1766-
* conflict with the lease we're trying to set.
1767-
*/
1768-
static int
1769-
check_conflicting_open(struct file *filp, const int arg, int flags)
1770-
{
1771-
struct inode *inode = file_inode(filp);
1772-
int self_wcount = 0, self_rcount = 0;
1773-
1774-
if (flags & FL_LAYOUT)
1775-
return 0;
1776-
if (flags & FL_DELEG)
1777-
/* We leave these checks to the caller */
1778-
return 0;
1779-
1780-
if (arg == F_RDLCK)
1781-
return inode_is_open_for_write(inode) ? -EAGAIN : 0;
1782-
else if (arg != F_WRLCK)
1783-
return 0;
1784-
1785-
/*
1786-
* Make sure that only read/write count is from lease requestor.
1787-
* Note that this will result in denying write leases when i_writecount
1788-
* is negative, which is what we want. (We shouldn't grant write leases
1789-
* on files open for execution.)
1790-
*/
1791-
if (filp->f_mode & FMODE_WRITE)
1792-
self_wcount = 1;
1793-
else if (filp->f_mode & FMODE_READ)
1794-
self_rcount = 1;
1795-
1796-
if (atomic_read(&inode->i_writecount) != self_wcount ||
1797-
atomic_read(&inode->i_readcount) != self_rcount)
1798-
return -EAGAIN;
1799-
1800-
return 0;
1801-
}
1802-
18031797
static int
18041798
generic_add_lease(struct file *filp, int arg, struct file_lease **flp, void **priv)
18051799
{
@@ -1836,7 +1830,7 @@ generic_add_lease(struct file *filp, int arg, struct file_lease **flp, void **pr
18361830
percpu_down_read(&file_rwsem);
18371831
spin_lock(&ctx->flc_lock);
18381832
time_out_leases(inode, &dispose);
1839-
error = check_conflicting_open(filp, arg, lease->c.flc_flags);
1833+
error = lease->fl_lmops->lm_open_conflict(filp, arg);
18401834
if (error)
18411835
goto out;
18421836

@@ -1893,7 +1887,7 @@ generic_add_lease(struct file *filp, int arg, struct file_lease **flp, void **pr
18931887
* precedes these checks.
18941888
*/
18951889
smp_mb();
1896-
error = check_conflicting_open(filp, arg, lease->c.flc_flags);
1890+
error = lease->fl_lmops->lm_open_conflict(filp, arg);
18971891
if (error) {
18981892
locks_unlink_lock_ctx(&lease->c);
18991893
goto out;

fs/nfsd/nfs4layouts.c

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -764,9 +764,28 @@ nfsd4_layout_lm_change(struct file_lease *onlist, int arg,
764764
return lease_modify(onlist, arg, dispose);
765765
}
766766

767+
/**
768+
* nfsd4_layout_lm_open_conflict - see if the given file points to an inode that has
769+
* an existing open that would conflict with the
770+
* desired lease.
771+
* @filp: file to check
772+
* @arg: type of lease that we're trying to acquire
773+
*
774+
* The kernel will call into this operation to determine whether there
775+
* are conflicting opens that may prevent the layout from being granted.
776+
* For nfsd, that check is done at a higher level, so this trivially
777+
* returns 0.
778+
*/
779+
static int
780+
nfsd4_layout_lm_open_conflict(struct file *filp, int arg)
781+
{
782+
return 0;
783+
}
784+
767785
static const struct lease_manager_operations nfsd4_layouts_lm_ops = {
768-
.lm_break = nfsd4_layout_lm_break,
769-
.lm_change = nfsd4_layout_lm_change,
786+
.lm_break = nfsd4_layout_lm_break,
787+
.lm_change = nfsd4_layout_lm_change,
788+
.lm_open_conflict = nfsd4_layout_lm_open_conflict,
770789
};
771790

772791
int

fs/nfsd/nfs4state.c

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5552,10 +5552,29 @@ nfsd_change_deleg_cb(struct file_lease *onlist, int arg,
55525552
return -EAGAIN;
55535553
}
55545554

5555+
/**
5556+
* nfsd4_deleg_lm_open_conflict - see if the given file points to an inode that has
5557+
* an existing open that would conflict with the
5558+
* desired lease.
5559+
* @filp: file to check
5560+
* @arg: type of lease that we're trying to acquire
5561+
*
5562+
* The kernel will call into this operation to determine whether there
5563+
* are conflicting opens that may prevent the deleg from being granted.
5564+
* For nfsd, that check is done at a higher level, so this trivially
5565+
* returns 0.
5566+
*/
5567+
static int
5568+
nfsd4_deleg_lm_open_conflict(struct file *filp, int arg)
5569+
{
5570+
return 0;
5571+
}
5572+
55555573
static const struct lease_manager_operations nfsd_lease_mng_ops = {
55565574
.lm_breaker_owns_lease = nfsd_breaker_owns_lease,
55575575
.lm_break = nfsd_break_deleg_cb,
55585576
.lm_change = nfsd_change_deleg_cb,
5577+
.lm_open_conflict = nfsd4_deleg_lm_open_conflict,
55595578
};
55605579

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

include/linux/filelock.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ struct lease_manager_operations {
4949
int (*lm_change)(struct file_lease *, int, struct list_head *);
5050
void (*lm_setup)(struct file_lease *, void **);
5151
bool (*lm_breaker_owns_lease)(struct file_lease *);
52+
int (*lm_open_conflict)(struct file *, int);
5253
};
5354

5455
struct lock_manager {

0 commit comments

Comments
 (0)