[PATCH] NFSD: restart ssc_expire_umount walk after dropping nfsd_ssc_lock

[Michael Bommarito]

nfsd4_ssc_expire_umount() walks nn->nfsd_ssc_mount_list with
list_for_each_entry_safe(ni, tmp, ...).  For each expired entry it
sets nsui_busy = true, drops nfsd_ssc_lock to run mntput() on the
source vfsmount, then reacquires the lock to list_del + kfree the
entry and continue iterating via the macro's saved tmp pointer.

The nsui_busy flag protects the current ni from concurrent
nfsd4_ssc_setup_dul() finders during the lock-drop window, but it
does not pin tmp.  Another nfsd RPC thread that fails its source-
server mount and reaches nfsd4_ssc_cancel_dul() will, during that
same window, take nfsd_ssc_lock, list_del + kfree its own ssc_umount
item, and release the lock.  If that item is the saved tmp of the
expire walk, the next iteration dereferences a freed
nfsd4_ssc_umount_item.

Reachability: triggered by any authenticated NFSv4.2 client that
can issue OP_COPY with cna_src.nl4_type = NL4_SERVER to a destination
nfsd built with CONFIG_NFSD_V4_2_INTER_SSC=y and started with
inter_copy_offload_enable=Y.  The client chooses the source-server
netaddr and can pick one that fails vfs_kern_mount() (unreachable,
RST after EXCHANGE_ID, etc.) to drive nfsd4_ssc_cancel_dul() into
the laundromat's lock-drop window.  Default Linux nfsd ships with
inter_copy_offload_enable=N, so the bug is reachable only on servers
where the administrator has explicitly opted into inter-SSC offload.

Restart the walk from the head after the mntput() unlock window so
no saved next pointer survives the lock-drop.  The list is bounded
by the number of active inter-server source mounts (typically small)
and the expire delayed-work runs periodically rather than per-IO,
so the restart is cheap.

Cc: stable@xxxxxxxxxxxxxxx
Fixes: f4e44b393389 ("NFSD: delay unmount source's export after inter-server copy completed.")
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@xxxxxxxxx>
---
 fs/nfsd/nfs4state.c | 45 ++++++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 19 deletions(-)

Reproduced under QEMU/KVM with KASAN, three nfsd network namespaces
on a single host so the kernel client treats them as distinct
servers, and Linux fault injection forcing vfs_kern_mount allocations
inside the destination nfsd to fail.  This drives nfsd4_ssc_cancel_dul
into a tight loop concurrent with the laundromat workqueue.

Stock kernel:

  BUG: KASAN: slab-use-after-free in laundromat_main+0x1756/0x1be0
  Read of size 8 at addr ffff88800ce9b200 by task kworker/u16:3
  Workqueue: nfsd4 laundromat_main

  Allocated by task 229:
   nfsd4_interssc_connect+0x3f5/0xd90    (nfsd4_ssc_setup_dul, inlined)
   nfsd4_copy+0x117d/0x1a30
   nfsd4_proc_compound+0xbe9/0x23f0

  Freed by task 229:
   kfree+0x18f/0x520
   nfsd4_interssc_connect+0xaff/0xd90    (nfsd4_ssc_cancel_dul, inlined)
   nfsd4_copy+0x117d/0x1a30

  The buggy address belongs to the cache kmalloc-128 of size 128.
  Kernel panic - not syncing: Fatal exception

Patched kernel ran the equivalent workload to completion with the
inter-SSC code path exercised 21-22 times per run and no KASAN
report.

The fault-injection knobs are standard Linux testing infrastructure
(see Documentation/fault-injection/) exercising the existing failure
path in nfsd; no kernel source was modified.  The same primitive
class was previously addressed by the OPEN-error path fix in
__nfs42_ssc_open(); this patch closes the corresponding hole in
the laundromat-driven delayed-unmount path.


diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 6b9c399b89dfb..03582f15e3e7e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6728,30 +6728,37 @@ static void nfsd4_ssc_shutdown_umount(struct nfsd_net *nn)
 static void nfsd4_ssc_expire_umount(struct nfsd_net *nn)
 {
 	bool do_wakeup = false;
-	struct nfsd4_ssc_umount_item *ni = NULL;
-	struct nfsd4_ssc_umount_item *tmp;
+	struct nfsd4_ssc_umount_item *ni;
 
+restart:
 	spin_lock(&nn->nfsd_ssc_lock);
-	list_for_each_entry_safe(ni, tmp, &nn->nfsd_ssc_mount_list, nsui_list) {
-		if (time_after(jiffies, ni->nsui_expire)) {
-			if (refcount_read(&ni->nsui_refcnt) > 1)
-				continue;
+	list_for_each_entry(ni, &nn->nfsd_ssc_mount_list, nsui_list) {
+		if (!time_after(jiffies, ni->nsui_expire))
+			break;
+		if (refcount_read(&ni->nsui_refcnt) > 1)
+			continue;
 
-			/* mark being unmount */
-			ni->nsui_busy = true;
-			spin_unlock(&nn->nfsd_ssc_lock);
-			mntput(ni->nsui_vfsmount);
-			spin_lock(&nn->nfsd_ssc_lock);
+		/* mark being unmount */
+		ni->nsui_busy = true;
+		spin_unlock(&nn->nfsd_ssc_lock);
+		mntput(ni->nsui_vfsmount);
+		spin_lock(&nn->nfsd_ssc_lock);
 
-			/* waiters need to start from begin of list */
-			list_del(&ni->nsui_list);
-			kfree(ni);
+		/* waiters need to start from begin of list */
+		list_del(&ni->nsui_list);
+		kfree(ni);
 
-			/* wakeup ssc_connect waiters */
-			do_wakeup = true;
-			continue;
-		}
-		break;
+		/* wakeup ssc_connect waiters */
+		do_wakeup = true;
+		/*
+		 * The list_for_each_entry_safe() saved-next pointer was
+		 * not pinned across the spin_unlock() above: a concurrent
+		 * nfsd4_ssc_cancel_dul() can free the next item under the
+		 * same spinlock while mntput() runs.  Restart the walk
+		 * from the head so no stale next is dereferenced.
+		 */
+		spin_unlock(&nn->nfsd_ssc_lock);
+		goto restart;
 	}
 	if (do_wakeup)
 		wake_up_all(&nn->nfsd_ssc_waitq);
-- 
2.53.0