From: Greg Kroah-Hartman Date: Tue, 20 Feb 2024 15:03:29 +0000 (+0100) Subject: 6.7-stable patches X-Git-Tag: v4.19.307~59 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b0d77915f87b0b8fa61151e851ac26c8c0eb4a12;p=thirdparty%2Fkernel%2Fstable-queue.git 6.7-stable patches added patches: nfsd-don-t-take-fi_lock-in-nfsd_break_deleg_cb.patch --- diff --git a/queue-6.7/nfsd-don-t-take-fi_lock-in-nfsd_break_deleg_cb.patch b/queue-6.7/nfsd-don-t-take-fi_lock-in-nfsd_break_deleg_cb.patch new file mode 100644 index 00000000000..2edda0a612a --- /dev/null +++ b/queue-6.7/nfsd-don-t-take-fi_lock-in-nfsd_break_deleg_cb.patch @@ -0,0 +1,92 @@ +From 5ea9a7c5fe4149f165f0e3b624fe08df02b6c301 Mon Sep 17 00:00:00 2001 +From: NeilBrown +Date: Mon, 5 Feb 2024 13:22:39 +1100 +Subject: nfsd: don't take fi_lock in nfsd_break_deleg_cb() + +From: NeilBrown + +commit 5ea9a7c5fe4149f165f0e3b624fe08df02b6c301 upstream. + +A recent change to check_for_locks() changed it to take ->flc_lock while +holding ->fi_lock. This creates a lock inversion (reported by lockdep) +because there is a case where ->fi_lock is taken while holding +->flc_lock. + +->flc_lock is held across ->fl_lmops callbacks, and +nfsd_break_deleg_cb() is one of those and does take ->fi_lock. However +it doesn't need to. + +Prior to v4.17-rc1~110^2~22 ("nfsd: create a separate lease for each +delegation") nfsd_break_deleg_cb() would walk the ->fi_delegations list +and so needed the lock. Since then it doesn't walk the list and doesn't +need the lock. + +Two actions are performed under the lock. One is to call +nfsd_break_one_deleg which calls nfsd4_run_cb(). These doesn't act on +the nfs4_file at all, so don't need the lock. + +The other is to set ->fi_had_conflict which is in the nfs4_file. +This field is only ever set here (except when initialised to false) +so there is no possible problem will multiple threads racing when +setting it. + +The field is tested twice in nfs4_set_delegation(). The first test does +not hold a lock and is documented as an opportunistic optimisation, so +it doesn't impose any need to hold ->fi_lock while setting +->fi_had_conflict. + +The second test in nfs4_set_delegation() *is* make under ->fi_lock, so +removing the locking when ->fi_had_conflict is set could make a change. +The change could only be interesting if ->fi_had_conflict tested as +false even though nfsd_break_one_deleg() ran before ->fi_lock was +unlocked. i.e. while hash_delegation_locked() was running. +As hash_delegation_lock() doesn't interact in any way with nfs4_run_cb() +there can be no importance to this interaction. + +So this patch removes the locking from nfsd_break_one_deleg() and moves +the final test on ->fi_had_conflict out of the locked region to make it +clear that locking isn't important to the test. It is still tested +*after* vfs_setlease() has succeeded. This might be significant and as +vfs_setlease() takes ->flc_lock, and nfsd_break_one_deleg() is called +under ->flc_lock this "after" is a true ordering provided by a spinlock. + +Fixes: edcf9725150e ("nfsd: fix RELEASE_LOCKOWNER") +Signed-off-by: NeilBrown +Reviewed-by: Jeff Layton +Signed-off-by: Chuck Lever +Signed-off-by: Greg Kroah-Hartman +--- + fs/nfsd/nfs4state.c | 11 +++++------ + 1 file changed, 5 insertions(+), 6 deletions(-) + +--- a/fs/nfsd/nfs4state.c ++++ b/fs/nfsd/nfs4state.c +@@ -4945,10 +4945,8 @@ nfsd_break_deleg_cb(struct file_lock *fl + */ + fl->fl_break_time = 0; + +- spin_lock(&fp->fi_lock); + fp->fi_had_conflict = true; + nfsd_break_one_deleg(dp); +- spin_unlock(&fp->fi_lock); + return false; + } + +@@ -5557,12 +5555,13 @@ nfs4_set_delegation(struct nfsd4_open *o + if (status) + goto out_unlock; + ++ status = -EAGAIN; ++ if (fp->fi_had_conflict) ++ goto out_unlock; ++ + spin_lock(&state_lock); + spin_lock(&fp->fi_lock); +- if (fp->fi_had_conflict) +- status = -EAGAIN; +- else +- status = hash_delegation_locked(dp, fp); ++ status = hash_delegation_locked(dp, fp); + spin_unlock(&fp->fi_lock); + spin_unlock(&state_lock); + diff --git a/queue-6.7/series b/queue-6.7/series index 8710d86851e..fe4fbc9ad8c 100644 --- a/queue-6.7/series +++ b/queue-6.7/series @@ -308,3 +308,4 @@ block-fix-partial-zone-append-completion-handling-in.patch usb-typec-tpcm-fix-issues-with-power-being-removed-during-reset.patch netfilter-ipset-fix-performance-regression-in-swap-operation.patch netfilter-ipset-missing-gc-cancellations-fixed.patch +nfsd-don-t-take-fi_lock-in-nfsd_break_deleg_cb.patch