[PATCH v3 0/2] lockd: Fix two teardown races in the block retry loop

[PATCH v3 0/2] lockd: Fix two teardown races in the block retry loop

[Chuck Lever]

nlmsvc_retry_blocked() is the only path that operates on a block with
nlm_blocked_lock dropped. Both defects live in that window. Shuangpeng
Bai's KASAN report catches one of them, a block freed while the retry
is still using it. sashiko-bot flagged the other while reviewing v1.
The retry re-queues a block that the teardown scan has already
retired, so the host that block pins is never reaped.

Patch 1 fixes the first and applies without patch 2, so it can be
backported on its own. Patch 2 is meant for mainline only. The race
it closes leaks a host, and closing it widens the f_mutex hold across
the retry loop.

v2 added a B_DEAD flag that makes nlmsvc_insert_block() refuse to
queue a retired block. That appeared to repair the second race, but
it doesn't work. Every caller relies on that insert to take the list
reference, so a refused insert lets nlmsvc_grant_blocked() drop the
last reference to a block whose file_lock the VFS still has queued
on a blocker.

Signed-off-by: Chuck Lever <cel@kernel.org>

---
Changes in v3:
- Split the fix in two. Patch 1 is the block reference alone.
- Drop B_DEAD. A refused insert could free a live block (sashiko-bot).
- Serialize the retry against nlmsvc_traverse_blocks() with f_mutex.
- Link to v2: https://lore.kernel.org/r/20260820150831.3291262-1-cel@kernel.org

Changes in v2:
- Add B_DEAD so a retry cannot requeue a retired block (sashiko-bot).
- Link to v1: https://lore.kernel.org/r/20260819162247.2970703-1-cel@kernel.org

---
Chuck Lever (2):
      lockd: Fix use-after-free in nlmsvc_retry_blocked
      lockd: Serialize block retries against host teardown

 fs/lockd/svclock.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)
---
base-commit: 01c2994ccb0197cb44b0db89aacab460110f6347
change-id: 20260820-lockd-retry-blocked-uaf-68f6b50098d9

Best regards,
--  
Chuck Lever

[PATCH v3 1/2] lockd: Fix use-after-free in nlmsvc_retry_blocked

[Chuck Lever]

nlmsvc_retry_blocked() examines the block at the head of nlm_blocked
under nlm_blocked_lock, then releases the lock before calling
nlmsvc_grant_blocked() or retry_deferred_block(). The nlm_blocked list
reference is all that keeps the block alive across that window.
nlmsvc_grant_blocked() does take one of its own, but not until after
the lock has been dropped. Unmounting the nfsd filesystem while a lock
request is still blocked reaches nlmsvc_traverse_blocks(), which drops
the list reference and frees the block along with the nlm_rqst hanging
off it.

  BUG: KASAN: slab-use-after-free in nlm_async_call+0xd6/0x230
  Read of size 8 at addr ffff88811b04c808 by task lockd/8377
    nlm_async_call+0xd6/0x230
    nlmsvc_retry_blocked+0x61c/0x800
    lockd+0x144/0x1c0
  Freed by task 8392:
    nlmsvc_release_block+0x231/0x290
    nlmsvc_traverse_blocks+0x139/0x1b0
    nlm_traverse_files+0x1aa/0xa00
    nlmsvc_free_host_resources+0x12/0x60
    nlm_shutdown_hosts_net+0x127/0x280
    lockd_down+0xd5/0x1c0

Take a reference before releasing nlm_blocked_lock and drop it once
the retry has run.

Fixes: 0e4ac9d93515 ("lockd: handle fl_grant callbacks")
Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
Closes: https://lore.kernel.org/linux-nfs/20260818235808.3458075-1-shuangpeng.kernel@gmail.com/
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/lockd/svclock.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index e628b5d35507..8d83283d3e21 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -1023,6 +1023,7 @@ nlmsvc_retry_blocked(struct svc_rqst *rqstp)
 			timeout = block->b_when - jiffies;
 			break;
 		}
+		kref_get(&block->b_count);
 		spin_unlock(&nlm_blocked_lock);
 
 		dprintk("nlmsvc_retry_blocked(%p, when=%ld)\n",
@@ -1033,6 +1034,7 @@ nlmsvc_retry_blocked(struct svc_rqst *rqstp)
 			retry_deferred_block(block);
 		} else
 			nlmsvc_grant_blocked(block);
+		nlmsvc_release_block(block);
 		spin_lock(&nlm_blocked_lock);
 	}
 	spin_unlock(&nlm_blocked_lock);

-- 
2.54.0

[PATCH v3 2/2] lockd: Serialize block retries against host teardown

[Chuck Lever]

nlmsvc_grant_blocked() unlinks a block from nlm_blocked before it
retries the lock, then re-inserts it. nlmsvc_traverse_blocks() skips a
block that is not on nlm_blocked, so a teardown scan that runs during a
retry passes it by and the retry puts it back. The surviving block pins
its host. lockd warns that it could not shut down the host module, and
the host outlives its network namespace.

Hold the file's f_mutex across the retry, and extend the scan's hold
across its unlink, so a scan and a retry of the same file can no longer
interleave. Drop the mutex before releasing a block reference, since
the last put takes f_mutex. A retry that waited out a scan re-checks
under nlm_blocked_lock that its block is still queued and due.

Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260819162247.2970703-1-cel@kernel.org?part=1
Signed-off-by: Chuck Lever <cel@kernel.org>
---
 fs/lockd/svclock.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index 8d83283d3e21..495eacb3264f 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -295,14 +295,17 @@ void nlmsvc_traverse_blocks(struct nlm_host *host,
 	list_for_each_entry_safe(block, next, &file->f_blocks, b_flist) {
 		if (!match(block->b_host, host))
 			continue;
-		/* Do not destroy blocks that are not on
-		 * the global retry list - why? */
+		/*
+		 * nlmsvc_retry_blocked() holds f_mutex while the block
+		 * is off nlm_blocked, so a block off the list here has
+		 * been retired.
+		 */
 		if (list_empty(&block->b_list))
 			continue;
 		kref_get(&block->b_count);
 		spin_unlock(&nlm_blocked_lock);
-		mutex_unlock(&file->f_mutex);
 		nlmsvc_unlink_block(block);
+		mutex_unlock(&file->f_mutex);
 		nlmsvc_release_block(block);
 		goto restart;
 	}
@@ -1012,6 +1015,8 @@ nlmsvc_retry_blocked(struct svc_rqst *rqstp)
 {
 	unsigned long	timeout = MAX_SCHEDULE_TIMEOUT;
 	struct nlm_block *block;
+	struct nlm_file *file;
+	bool due;
 
 	spin_lock(&nlm_blocked_lock);
 	while (!list_empty(&nlm_blocked) && !svc_thread_should_stop(rqstp)) {
@@ -1026,6 +1031,25 @@ nlmsvc_retry_blocked(struct svc_rqst *rqstp)
 		kref_get(&block->b_count);
 		spin_unlock(&nlm_blocked_lock);
 
+		/*
+		 * Hold f_mutex so nlmsvc_traverse_blocks() cannot scan
+		 * the file while the retry has the block off nlm_blocked.
+		 */
+		file = block->b_file;
+		mutex_lock(&file->f_mutex);
+		spin_lock(&nlm_blocked_lock);
+		due = !list_empty(&block->b_list) &&
+		      block->b_when != NLM_NEVER &&
+		      !time_after(block->b_when, jiffies);
+		spin_unlock(&nlm_blocked_lock);
+
+		if (!due) {
+			mutex_unlock(&file->f_mutex);
+			nlmsvc_release_block(block);
+			spin_lock(&nlm_blocked_lock);
+			continue;
+		}
+
 		dprintk("nlmsvc_retry_blocked(%p, when=%ld)\n",
 			block, block->b_when);
 		if (block->b_flags & B_QUEUED) {
@@ -1034,6 +1058,7 @@ nlmsvc_retry_blocked(struct svc_rqst *rqstp)
 			retry_deferred_block(block);
 		} else
 			nlmsvc_grant_blocked(block);
+		mutex_unlock(&file->f_mutex);
 		nlmsvc_release_block(block);
 		spin_lock(&nlm_blocked_lock);
 	}

-- 
2.54.0

Re: [PATCH v3 0/2] lockd: Fix two teardown races in the block retry loop

[Jeff Layton]

On Sat, 2026-08-22 at 21:40 -0400, Chuck Lever wrote:
> nlmsvc_retry_blocked() is the only path that operates on a block with
> nlm_blocked_lock dropped. Both defects live in that window. Shuangpeng
> Bai's KASAN report catches one of them, a block freed while the retry
> is still using it. sashiko-bot flagged the other while reviewing v1.
> The retry re-queues a block that the teardown scan has already
> retired, so the host that block pins is never reaped.
> 
> Patch 1 fixes the first and applies without patch 2, so it can be
> backported on its own. Patch 2 is meant for mainline only. The race
> it closes leaks a host, and closing it widens the f_mutex hold across
> the retry loop.
> 
> v2 added a B_DEAD flag that makes nlmsvc_insert_block() refuse to
> queue a retired block. That appeared to repair the second race, but
> it doesn't work. Every caller relies on that insert to take the list
> reference, so a refused insert lets nlmsvc_grant_blocked() drop the
> last reference to a block whose file_lock the VFS still has queued
> on a blocker.
> 
> Signed-off-by: Chuck Lever <cel@kernel.org>
> 
> ---
> Changes in v3:
> - Split the fix in two. Patch 1 is the block reference alone.
> - Drop B_DEAD. A refused insert could free a live block (sashiko-bot).
> - Serialize the retry against nlmsvc_traverse_blocks() with f_mutex.
> - Link to v2: https://lore.kernel.org/r/20260820150831.3291262-1-cel@kernel.org
> 
> Changes in v2:
> - Add B_DEAD so a retry cannot requeue a retired block (sashiko-bot).
> - Link to v1: https://lore.kernel.org/r/20260819162247.2970703-1-cel@kernel.org
> 
> ---
> Chuck Lever (2):
>       lockd: Fix use-after-free in nlmsvc_retry_blocked
>       lockd: Serialize block retries against host teardown
> 
>  fs/lockd/svclock.c | 33 ++++++++++++++++++++++++++++++---
>  1 file changed, 30 insertions(+), 3 deletions(-)
> ---
> base-commit: 01c2994ccb0197cb44b0db89aacab460110f6347
> change-id: 20260820-lockd-retry-blocked-uaf-68f6b50098d9
> 
> Best regards,
> --  
> Chuck Lever

Reviewed-by: Jeff Layton <jlayton@kernel.org>