From: Sasha Levin Date: Sun, 10 Mar 2024 13:21:47 +0000 (-0400) Subject: Drop nfsd-fix-release_lockowner.patch X-Git-Tag: v6.8.1~20 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=cce5f5f00caf42e1d1780fe08aa279c7b7b98906;p=thirdparty%2Fkernel%2Fstable-queue.git Drop nfsd-fix-release_lockowner.patch Signed-off-by: Sasha Levin --- diff --git a/queue-5.10/nfsd-fix-release_lockowner.patch b/queue-5.10/nfsd-fix-release_lockowner.patch deleted file mode 100644 index 2b255c7fb62..00000000000 --- a/queue-5.10/nfsd-fix-release_lockowner.patch +++ /dev/null @@ -1,149 +0,0 @@ -From 094bb06a555bffa2d5058ea6657fea919095e171 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 b6480be7b5e6a..16b073c637986 100644 ---- a/fs/nfsd/nfs4state.c -+++ b/fs/nfsd/nfs4state.c -@@ -7080,14 +7080,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); -@@ -7103,7 +7105,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; - } - -@@ -7113,10 +7116,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 -@@ -7152,10 +7153,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.10/series b/queue-5.10/series index 72bb883bc2d..e6c946da502 100644 --- a/queue-5.10/series +++ b/queue-5.10/series @@ -29,7 +29,6 @@ netrom-fix-a-data-race-around-sysctl_netrom_routing_.patch netrom-fix-a-data-race-around-sysctl_netrom_link_fai.patch netrom-fix-data-races-around-sysctl_net_busy_read.patch nfsd-modernize-nfsd4_release_lockowner.patch -nfsd-fix-release_lockowner.patch selftests-mm-switch-to-bash-from-sh.patch selftests-mm-fix-map_hugetlb-failure-on-64k-page-siz.patch um-allow-not-setting-extra-rpaths-in-the-linux-binar.patch diff --git a/queue-5.15/nfsd-fix-release_lockowner.patch b/queue-5.15/nfsd-fix-release_lockowner.patch deleted file mode 100644 index 90a1edf918b..00000000000 --- a/queue-5.15/nfsd-fix-release_lockowner.patch +++ /dev/null @@ -1,149 +0,0 @@ -From ac6bed871ef46355b3aff79f487d6b7da525587d 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 f8533299db1c5..07ea358036293 100644 ---- a/fs/nfsd/nfs4state.c -+++ b/fs/nfsd/nfs4state.c -@@ -7257,14 +7257,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); -@@ -7280,7 +7282,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; - } - -@@ -7290,10 +7293,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 -@@ -7329,10 +7330,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.15/series b/queue-5.15/series index 28372652949..7a22caf8713 100644 --- a/queue-5.15/series +++ b/queue-5.15/series @@ -27,7 +27,6 @@ netrom-fix-a-data-race-around-sysctl_netrom_routing_.patch netrom-fix-a-data-race-around-sysctl_netrom_link_fai.patch netrom-fix-data-races-around-sysctl_net_busy_read.patch nfsd-modernize-nfsd4_release_lockowner.patch -nfsd-fix-release_lockowner.patch alsa-usb-audio-refcount-multiple-accesses-on-the-sin.patch alsa-usb-audio-clear-fixed-clock-rate-at-closing-ep.patch alsa-usb-audio-split-endpoint-setups-for-hw_params-a.patch diff --git a/queue-5.4/nfsd-fix-release_lockowner.patch b/queue-5.4/nfsd-fix-release_lockowner.patch deleted file mode 100644 index 89329d225f1..00000000000 --- a/queue-5.4/nfsd-fix-release_lockowner.patch +++ /dev/null @@ -1,149 +0,0 @@ -From ac66408be68cc89cfe278cfd29c3bdfa7efbb17e 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 0dfc45d376587..bca22325083c0 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/series b/queue-5.4/series index 770e7d64693..58f7a14fc15 100644 --- a/queue-5.4/series +++ b/queue-5.4/series @@ -23,7 +23,6 @@ netrom-fix-a-data-race-around-sysctl_netrom_routing_.patch netrom-fix-a-data-race-around-sysctl_netrom_link_fai.patch netrom-fix-data-races-around-sysctl_net_busy_read.patch nfsd-modernize-nfsd4_release_lockowner.patch -nfsd-fix-release_lockowner.patch selftests-mm-fix-map_hugetlb-failure-on-64k-page-siz.patch um-allow-not-setting-extra-rpaths-in-the-linux-binar.patch um-fix-adding-no-pie-for-clang.patch