[PATCH] lockd: fix use-after-free in nlmsvc_retry_blocked()

[PATCH] lockd: fix use-after-free in nlmsvc_retry_blocked()

[Abdifatah Suruur]

A block queued on the nlm_blocked list holds exactly one kref, the
list's: nlmsvc_create_block() hands out an initial reference,
nlmsvc_insert_block_locked() takes the list reference, and
nlmsvc_lock() releases the initial one at out:.

nlmsvc_retry_blocked() then drops nlm_blocked_lock and dereferences
`block` (b_when, b_flags, b_deferred_req) and passes it to
retry_deferred_block() or nlmsvc_grant_blocked() without holding any
reference of its own.  Concurrently, an svc thread processing the
client's GRANT_RES (nlmsvc_grant_reply()), a CANCEL or UNLOCK
(nlmsvc_cancel_blocked()), or a host failover sweep
(nlmsvc_traverse_blocks()) can find the same block, unlink it and drop
the last kref, freeing it while the lockd kthread is still using the
pointer.  The freed slab is then written through: kref_get() on the
freed block, the B_TIMED_OUT flag, the list operations in
nlmsvc_insert_block(), and the b_deferred_req revisit - a
use-after-free on a remotely reachable path.

nlmsvc_notify_blocked() has the same problem: it moves the block to
the head of the list and then calls svc_wake_up(block->b_daemon)
after dropping nlm_blocked_lock.  Keep that wake-up under the
spinlock, as nlmsvc_grant_deferred() already does.

Pin the block before dropping the spinlock in nlmsvc_retry_blocked()
and release the pin after processing, so the block cannot be freed
while it is in use.

Cc: stable@vger.kernel.org
Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
---
 fs/lockd/svclock.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/lockd/svclock.c b/fs/lockd/svclock.c
index e628b5d355071..38d02b10591ae 100644
--- a/fs/lockd/svclock.c
+++ b/fs/lockd/svclock.c
@@ -775,8 +775,8 @@ nlmsvc_notify_blocked(struct file_lock *fl)
 	list_for_each_entry(block, &nlm_blocked, b_list) {
 		if (nlm_compare_locks(&block->b_call->a_args.lock.fl, fl)) {
 			nlmsvc_insert_block_locked(block, 0);
-			spin_unlock(&nlm_blocked_lock);
 			svc_wake_up(block->b_daemon);
+			spin_unlock(&nlm_blocked_lock);
 			return;
 		}
 	}
@@ -1023,6 +1023,13 @@ nlmsvc_retry_blocked(struct svc_rqst *rqstp)
 			timeout = block->b_when - jiffies;
 			break;
 		}
+		/*
+		 * Pin the block before dropping nlm_blocked_lock: a
+		 * concurrent GRANT_RES, CANCEL or UNLOCK can unlink the
+		 * block and drop the last kref, freeing it while we are
+		 * still using it.
+		 */
+		kref_get(&block->b_count);
 		spin_unlock(&nlm_blocked_lock);
 
 		dprintk("nlmsvc_retry_blocked(%p, when=%ld)\n",
@@ -1033,6 +1040,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.53.0

Re: [PATCH] lockd: fix use-after-free in nlmsvc_retry_blocked()

[Chuck Lever]

Hi Abdifatah,

Thanks for looking at this code. I'm not going to take this patch,
for the reasons below.

On Wed, Sep 30, 2026 at 11:22:12PM +0300, Abdifatah Suruur wrote:
> Pin the block before dropping the spinlock in nlmsvc_retry_blocked()
> and release the pin after processing, so the block cannot be freed
> while it is in use.

This part is already fixed. The nfsd-testing branch carries

  lockd: Fix use-after-free in nlmsvc_retry_blocked
  lockd: Serialize block retries against host teardown

The first takes the same reference in the same place, so your patch
no longer applies there. For lockd and NFSD work, please base patches
on the nfsd-testing branch of

  git://git.kernel.org/pub/scm/linux/kernel/git/cel/linux.git

> nlmsvc_notify_blocked() has the same problem: it moves the block to
> the head of the list and then calls svc_wake_up(block->b_daemon)
> after dropping nlm_blocked_lock.  Keep that wake-up under the
> spinlock, as nlmsvc_grant_deferred() already does.

This one appears to be unreachable.

nlmsvc_notify_blocked() is the lm_notify callback. It is called from
__locks_wake_up_blocks() with blocked_lock_lock held, and that
function clears waiter->flc_blocker only after lm_notify returns.

The block is on nlm_blocked when the callback finds it, so the list
holds a reference. A block whose file_lock is waiting comes off that
list through nlmsvc_unlink_block(), which calls locks_delete_block()
on the block's file_lock before it calls nlmsvc_remove_block(). That
covers GRANT_RES, CANCEL, nlmsvc_traverse_blocks(), and
nlmsvc_grant_blocked().

While the callback is running, flc_blocker is still set, so
locks_delete_block() cannot take its lockless early return. It has to
acquire blocked_lock_lock, and it waits there until the callback has
returned. The list reference therefore cannot be dropped between the
spin_unlock() and the svc_wake_up().


-- 
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)