From: Sasha Levin Date: Mon, 29 Jan 2024 00:17:59 +0000 (-0500) Subject: Fixes for 5.4 X-Git-Tag: v6.1.76~22 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=449abc02ef32a6c5ec62b4d054e7111f2693aea4;p=thirdparty%2Fkernel%2Fstable-queue.git Fixes for 5.4 Signed-off-by: Sasha Levin --- diff --git a/queue-5.4/nfsd-add-documenting-comment-for-nfsd4_release_locko.patch b/queue-5.4/nfsd-add-documenting-comment-for-nfsd4_release_locko.patch new file mode 100644 index 00000000000..a6c6b774159 --- /dev/null +++ b/queue-5.4/nfsd-add-documenting-comment-for-nfsd4_release_locko.patch @@ -0,0 +1,73 @@ +From b0b0ed9987913fc43f27c84225455f6fcac0e2ad Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Sun, 22 May 2022 12:34:38 -0400 +Subject: NFSD: Add documenting comment for nfsd4_release_lockowner() + +From: Chuck Lever + +[ Upstream commit 043862b09cc00273e35e6c3a6389957953a34207 ] + +And return explicit nfserr values that match what is documented in the +new comment / API contract. + +Signed-off-by: Chuck Lever +Stable-dep-of: edcf9725150e ("nfsd: fix RELEASE_LOCKOWNER") +Signed-off-by: Sasha Levin +--- + fs/nfsd/nfs4state.c | 23 ++++++++++++++++++++--- + 1 file changed, 20 insertions(+), 3 deletions(-) + +diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c +index 9a77a3eac4ac..0dfc45d37658 100644 +--- a/fs/nfsd/nfs4state.c ++++ b/fs/nfsd/nfs4state.c +@@ -6867,6 +6867,23 @@ check_for_locks(struct nfs4_file *fp, struct nfs4_lockowner *lowner) + return status; + } + ++/** ++ * nfsd4_release_lockowner - process NFSv4.0 RELEASE_LOCKOWNER operations ++ * @rqstp: RPC transaction ++ * @cstate: NFSv4 COMPOUND state ++ * @u: RELEASE_LOCKOWNER arguments ++ * ++ * The lockowner's so_count is bumped when a lock record is added ++ * or when copying a conflicting lock. The latter case is brief, ++ * but can lead to fleeting false positives when looking for ++ * locks-in-use. ++ * ++ * Return values: ++ * %nfs_ok: lockowner released or not found ++ * %nfserr_locks_held: lockowner still in use ++ * %nfserr_stale_clientid: clientid no longer active ++ * %nfserr_expired: clientid not recognized ++ */ + __be32 + nfsd4_release_lockowner(struct svc_rqst *rqstp, + struct nfsd4_compound_state *cstate, +@@ -6893,7 +6910,7 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp, + lo = find_lockowner_str_locked(clp, &rlockowner->rl_owner); + if (!lo) { + spin_unlock(&clp->cl_lock); +- return status; ++ return nfs_ok; + } + if (atomic_read(&lo->lo_owner.so_count) != 2) { + spin_unlock(&clp->cl_lock); +@@ -6909,11 +6926,11 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp, + put_ol_stateid_locked(stp, &reaplist); + } + spin_unlock(&clp->cl_lock); ++ + free_ol_stateid_reaplist(&reaplist); + remove_blocked_locks(lo); + nfs4_put_stateowner(&lo->lo_owner); +- +- return status; ++ return nfs_ok; + } + + static inline struct nfs4_client_reclaim * +-- +2.43.0 + diff --git a/queue-5.4/nfsd-fix-release_lockowner.patch b/queue-5.4/nfsd-fix-release_lockowner.patch new file mode 100644 index 00000000000..6ef9a949475 --- /dev/null +++ b/queue-5.4/nfsd-fix-release_lockowner.patch @@ -0,0 +1,149 @@ +From 9dc318f8e50bb1c547f2b480a1162c645eb7e9bc Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Mon, 22 Jan 2024 14:58:16 +1100 +Subject: nfsd: fix RELEASE_LOCKOWNER + +From: NeilBrown + +[ Upstream commit edcf9725150e42beeca42d085149f4c88fa97afd ] + +The test on so_count in nfsd4_release_lockowner() is nonsense and +harmful. Revert to using check_for_locks(), changing that to not sleep. + +First: harmful. +As is documented in the kdoc comment for nfsd4_release_lockowner(), the +test on so_count can transiently return a false positive resulting in a +return of NFS4ERR_LOCKS_HELD when in fact no locks are held. This is +clearly a protocol violation and with the Linux NFS client it can cause +incorrect behaviour. + +If RELEASE_LOCKOWNER is sent while some other thread is still +processing a LOCK request which failed because, at the time that request +was received, the given owner held a conflicting lock, then the nfsd +thread processing that LOCK request can hold a reference (conflock) to +the lock owner that causes nfsd4_release_lockowner() to return an +incorrect error. + +The Linux NFS client ignores that NFS4ERR_LOCKS_HELD error because it +never sends NFS4_RELEASE_LOCKOWNER without first releasing any locks, so +it knows that the error is impossible. It assumes the lock owner was in +fact released so it feels free to use the same lock owner identifier in +some later locking request. + +When it does reuse a lock owner identifier for which a previous RELEASE +failed, it will naturally use a lock_seqid of zero. However the server, +which didn't release the lock owner, will expect a larger lock_seqid and +so will respond with NFS4ERR_BAD_SEQID. + +So clearly it is harmful to allow a false positive, which testing +so_count allows. + +The test is nonsense because ... well... it doesn't mean anything. + +so_count is the sum of three different counts. +1/ the set of states listed on so_stateids +2/ the set of active vfs locks owned by any of those states +3/ various transient counts such as for conflicting locks. + +When it is tested against '2' it is clear that one of these is the +transient reference obtained by find_lockowner_str_locked(). It is not +clear what the other one is expected to be. + +In practice, the count is often 2 because there is precisely one state +on so_stateids. If there were more, this would fail. + +In my testing I see two circumstances when RELEASE_LOCKOWNER is called. +In one case, CLOSE is called before RELEASE_LOCKOWNER. That results in +all the lock states being removed, and so the lockowner being discarded +(it is removed when there are no more references which usually happens +when the lock state is discarded). When nfsd4_release_lockowner() finds +that the lock owner doesn't exist, it returns success. + +The other case shows an so_count of '2' and precisely one state listed +in so_stateid. It appears that the Linux client uses a separate lock +owner for each file resulting in one lock state per lock owner, so this +test on '2' is safe. For another client it might not be safe. + +So this patch changes check_for_locks() to use the (newish) +find_any_file_locked() so that it doesn't take a reference on the +nfs4_file and so never calls nfsd_file_put(), and so never sleeps. With +this check is it safe to restore the use of check_for_locks() rather +than testing so_count against the mysterious '2'. + +Fixes: ce3c4ad7f4ce ("NFSD: Fix possible sleep during nfsd4_release_lockowner()") +Signed-off-by: NeilBrown +Reviewed-by: Jeff Layton +Cc: stable@vger.kernel.org # v6.2+ +Signed-off-by: Chuck Lever +Signed-off-by: Sasha Levin +--- + fs/nfsd/nfs4state.c | 26 +++++++++++++++----------- + 1 file changed, 15 insertions(+), 11 deletions(-) + +diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c +index 0dfc45d37658..bca22325083c 100644 +--- a/fs/nfsd/nfs4state.c ++++ b/fs/nfsd/nfs4state.c +@@ -6840,14 +6840,16 @@ check_for_locks(struct nfs4_file *fp, struct nfs4_lockowner *lowner) + { + struct file_lock *fl; + int status = false; +- struct nfsd_file *nf = find_any_file(fp); ++ struct nfsd_file *nf; + struct inode *inode; + struct file_lock_context *flctx; + ++ spin_lock(&fp->fi_lock); ++ nf = find_any_file_locked(fp); + if (!nf) { + /* Any valid lock stateid should have some sort of access */ + WARN_ON_ONCE(1); +- return status; ++ goto out; + } + + inode = locks_inode(nf->nf_file); +@@ -6863,7 +6865,8 @@ check_for_locks(struct nfs4_file *fp, struct nfs4_lockowner *lowner) + } + spin_unlock(&flctx->flc_lock); + } +- nfsd_file_put(nf); ++out: ++ spin_unlock(&fp->fi_lock); + return status; + } + +@@ -6873,10 +6876,8 @@ check_for_locks(struct nfs4_file *fp, struct nfs4_lockowner *lowner) + * @cstate: NFSv4 COMPOUND state + * @u: RELEASE_LOCKOWNER arguments + * +- * The lockowner's so_count is bumped when a lock record is added +- * or when copying a conflicting lock. The latter case is brief, +- * but can lead to fleeting false positives when looking for +- * locks-in-use. ++ * Check if theree are any locks still held and if not - free the lockowner ++ * and any lock state that is owned. + * + * Return values: + * %nfs_ok: lockowner released or not found +@@ -6912,10 +6913,13 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp, + spin_unlock(&clp->cl_lock); + return nfs_ok; + } +- if (atomic_read(&lo->lo_owner.so_count) != 2) { +- spin_unlock(&clp->cl_lock); +- nfs4_put_stateowner(&lo->lo_owner); +- return nfserr_locks_held; ++ ++ list_for_each_entry(stp, &lo->lo_owner.so_stateids, st_perstateowner) { ++ if (check_for_locks(stp->st_stid.sc_file, lo)) { ++ spin_unlock(&clp->cl_lock); ++ nfs4_put_stateowner(&lo->lo_owner); ++ return nfserr_locks_held; ++ } + } + unhash_lockowner_locked(lo); + while (!list_empty(&lo->lo_owner.so_stateids)) { +-- +2.43.0 + diff --git a/queue-5.4/nfsd-modernize-nfsd4_release_lockowner.patch b/queue-5.4/nfsd-modernize-nfsd4_release_lockowner.patch new file mode 100644 index 00000000000..65a2d9be62e --- /dev/null +++ b/queue-5.4/nfsd-modernize-nfsd4_release_lockowner.patch @@ -0,0 +1,86 @@ +From 4f0fca8c3fa2d6fa3e875c131af1c4cfbb8125f7 Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Sun, 22 May 2022 12:07:18 -0400 +Subject: NFSD: Modernize nfsd4_release_lockowner() + +From: Chuck Lever + +[ Upstream commit bd8fdb6e545f950f4654a9a10d7e819ad48146e5 ] + +Refactor: Use existing helpers that other lock operations use. This +change removes several automatic variables, so re-organize the +variable declarations for readability. + +Signed-off-by: Chuck Lever +Stable-dep-of: edcf9725150e ("nfsd: fix RELEASE_LOCKOWNER") +Signed-off-by: Sasha Levin +--- + fs/nfsd/nfs4state.c | 36 +++++++++++------------------------- + 1 file changed, 11 insertions(+), 25 deletions(-) + +diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c +index a0aa7e63739d..9a77a3eac4ac 100644 +--- a/fs/nfsd/nfs4state.c ++++ b/fs/nfsd/nfs4state.c +@@ -6873,16 +6873,13 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp, + union nfsd4_op_u *u) + { + struct nfsd4_release_lockowner *rlockowner = &u->release_lockowner; ++ struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id); + clientid_t *clid = &rlockowner->rl_clientid; +- struct nfs4_stateowner *sop; +- struct nfs4_lockowner *lo = NULL; + struct nfs4_ol_stateid *stp; +- struct xdr_netobj *owner = &rlockowner->rl_owner; +- unsigned int hashval = ownerstr_hashval(owner); +- __be32 status; +- struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id); ++ struct nfs4_lockowner *lo; + struct nfs4_client *clp; +- LIST_HEAD (reaplist); ++ LIST_HEAD(reaplist); ++ __be32 status; + + dprintk("nfsd4_release_lockowner clientid: (%08x/%08x):\n", + clid->cl_boot, clid->cl_id); +@@ -6890,30 +6887,19 @@ nfsd4_release_lockowner(struct svc_rqst *rqstp, + status = lookup_clientid(clid, cstate, nn); + if (status) + return status; +- + clp = cstate->clp; +- /* Find the matching lock stateowner */ +- spin_lock(&clp->cl_lock); +- list_for_each_entry(sop, &clp->cl_ownerstr_hashtbl[hashval], +- so_strhash) { + +- if (sop->so_is_open_owner || !same_owner_str(sop, owner)) +- continue; +- +- if (atomic_read(&sop->so_count) != 1) { +- spin_unlock(&clp->cl_lock); +- return nfserr_locks_held; +- } +- +- lo = lockowner(sop); +- nfs4_get_stateowner(sop); +- break; +- } ++ spin_lock(&clp->cl_lock); ++ lo = find_lockowner_str_locked(clp, &rlockowner->rl_owner); + if (!lo) { + spin_unlock(&clp->cl_lock); + return status; + } +- ++ if (atomic_read(&lo->lo_owner.so_count) != 2) { ++ spin_unlock(&clp->cl_lock); ++ nfs4_put_stateowner(&lo->lo_owner); ++ return nfserr_locks_held; ++ } + unhash_lockowner_locked(lo); + while (!list_empty(&lo->lo_owner.so_stateids)) { + stp = list_first_entry(&lo->lo_owner.so_stateids, +-- +2.43.0 + diff --git a/queue-5.4/series b/queue-5.4/series index a515eaaad18..f6804bfd334 100644 --- a/queue-5.4/series +++ b/queue-5.4/series @@ -49,3 +49,6 @@ gpiolib-acpi-ignore-touchpad-wakeup-on-gpd-g1619-04.patch drm-don-t-unref-the-same-fb-many-times-by-mistake-due-to-deadlock-handling.patch drm-bridge-nxp-ptn3460-fix-i2c_master_send-error-checking.patch drm-bridge-nxp-ptn3460-simplify-some-error-checking.patch +nfsd-modernize-nfsd4_release_lockowner.patch +nfsd-add-documenting-comment-for-nfsd4_release_locko.patch +nfsd-fix-release_lockowner.patch