2021-06-14 14:49:46

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 0/3] reexport lock fixes

From: "J. Bruce Fields" <[email protected]>

The following fix up some problems that can cause crashes or silently
broken lock guarantees in the reexport case.

Not fixed:
- Attempts to reclaim locks after a reboot of the reexport
server will fail. This at least seems like an improvement
over the current situation, which is that they'll succeed even
in cases where they shouldn't. Complete support for reboot
recovery is a bigger job.

- NFSv4.1+ lock nofications don't work. So, clients have to
poll as they do with NFSv4.0, which is suboptimal, but correct
(and an improvement over the current situation, which is a
kernel oops).

So what we have at this point is a suboptimal lock implementation that
doesn't support lock recovery.

Another alternative might be to turn off file locking entirely in the
re-export case. I'd rather take the incremental improvement and fix the
oopses.

--b.

J. Bruce Fields (3):
nfs: don't atempt blocking locks on nfs reexports
lockd: lockd server-side shouldn't set fl_ops
nfs: don't allow reexport reclaims

fs/lockd/svclock.c | 30 ++++++++++++------------------
fs/nfs/export.c | 2 +-
fs/nfs/file.c | 3 +++
fs/nfsd/nfs4state.c | 11 +++++++++--
fs/nfsd/nfsproc.c | 1 +
include/linux/exportfs.h | 2 ++
include/linux/fs.h | 1 +
7 files changed, 29 insertions(+), 21 deletions(-)

--
2.31.1


2021-06-14 14:49:46

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 2/3] lockd: lockd server-side shouldn't set fl_ops

From: "J. Bruce Fields" <[email protected]>

Locks have two sets of op arrays, fl_lmops for the lock manager (lockd
or nfsd), fl_ops for the filesystem. The server-side lockd code has
been setting its own fl_ops, which leads to confusion (and crashes) in
the reexport case, where the filesystem expects to be the only one
setting fl_ops.

And there's no reason for it that I can see-the lm_get/put_owner ops do
the same job.

Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/lockd/svclock.c | 30 ++++++++++++------------------
1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index 61d3cc2283dc..1781fc5e9091 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -395,28 +395,10 @@ nlmsvc_release_lockowner(struct nlm_lock *lock)
nlmsvc_put_lockowner(lock->fl.fl_owner);
}

-static void nlmsvc_locks_copy_lock(struct file_lock *new, struct file_lock *fl)
-{
- struct nlm_lockowner *nlm_lo = (struct nlm_lockowner *)fl->fl_owner;
- new->fl_owner = nlmsvc_get_lockowner(nlm_lo);
-}
-
-static void nlmsvc_locks_release_private(struct file_lock *fl)
-{
- nlmsvc_put_lockowner((struct nlm_lockowner *)fl->fl_owner);
-}
-
-static const struct file_lock_operations nlmsvc_lock_ops = {
- .fl_copy_lock = nlmsvc_locks_copy_lock,
- .fl_release_private = nlmsvc_locks_release_private,
-};
-
void nlmsvc_locks_init_private(struct file_lock *fl, struct nlm_host *host,
pid_t pid)
{
fl->fl_owner = nlmsvc_find_lockowner(host, pid);
- if (fl->fl_owner != NULL)
- fl->fl_ops = &nlmsvc_lock_ops;
}

/*
@@ -788,9 +770,21 @@ nlmsvc_notify_blocked(struct file_lock *fl)
printk(KERN_WARNING "lockd: notification for unknown block!\n");
}

+static fl_owner_t nlmsvc_get_owner(fl_owner_t owner)
+{
+ return nlmsvc_get_lockowner(owner);
+}
+
+static void nlmsvc_put_owner(fl_owner_t owner)
+{
+ nlmsvc_put_lockowner(owner);
+}
+
const struct lock_manager_operations nlmsvc_lock_operations = {
.lm_notify = nlmsvc_notify_blocked,
.lm_grant = nlmsvc_grant_deferred,
+ .lm_get_owner = nlmsvc_get_owner,
+ .lm_put_owner = nlmsvc_put_owner,
};

/*
--
2.31.1