Re: [PATCH v3] nfsd: fix use-after-free of DRC slot in nfsd4_sequence()

[Ingo Molnar <mingo@kernel.org>]

* Jeff Layton <jlayton@kernel.org> wrote:

> I think this problem is already fixed (and more comprehensively) by
> this patch in Chuck's nfsd-testing branch:
> 
>     f5d22e372f4a nfsd: don't free session slots that are still in use

Commit/patch attached below for convenience - and it's now upstream
as well already.

Thanks,

	Ingo

=========================>
# From f5d22e372f4ac3eb287037a488c29691d52a6330 Mon Sep 17 00:00:00 2001
From: Jeff Layton <jlayton@kernel.org>
Date: Tue, 26 May 2026 12:24:48 -0400
Subject: [PATCH] nfsd: don't free session slots that are still in use

nfsd4_sequence() can free the very slot it is currently processing.
When the session shrinker has reduced se_target_maxslots below
se_fchannel.maxreqs, the shrink path checks three conditions before
calling free_session_slots():

  1. se_target_maxslots < maxreqs  (shrink was advertised)
  2. slot->sl_generation == se_slot_gen  (slot is up-to-date)
  3. seq->maxslots <= se_target_maxslots  (client acknowledges)

However, seq->slotid is never checked against se_target_maxslots.
A client using a slot in the range [se_target_maxslots, maxreqs) can
satisfy all three conditions: its slot has the current generation
(set by a prior SEQUENCE), and it sends sa_highest_slotid <=
se_target_maxslots to acknowledge the reduction.

free_session_slots() then kfrees every slot at index >=
se_target_maxslots, including the caller's own slot. The function
continues to write sl_seqid, sl_flags, sl_generation, and stores the
dangling pointer in cstate->slot. Later, nfsd4_store_cache_entry()
copies up to maxresp_cached bytes of the compound reply into the freed
sl_data[] array, corrupting whatever slab object now occupies that
address.

Additionally, a concurrent thread processing SEQUENCE on a different
high-numbered slot can have its slot freed out from under it.
NFSD4_SLOT_INUSE is set under nn->client_lock before the lock is
released, so any concurrent thread past SEQUENCE will have its slot
marked. However, free_session_slots() does not check NFSD4_SLOT_INUSE
before freeing.

Fix both problems by:
 1. Checking that the current request's slotid is below the shrink
    boundary.
 2. Scanning slots in the to-be-freed range for NFSD4_SLOT_INUSE and
    deferring the shrink if any are active.

Fixes: fc8738c68d0b ("nfsd: add support for freeing unused session-DRC slots")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260526-nfsd4_sequence_shrink_uaf_on_loaded_slot-v2-1-74a89db0639e@kernel.org
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs4state.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 8f90fba53357..0d6a7440a4e2 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4496,6 +4496,19 @@ static void nfsd4_construct_sequence_response(struct nfsd4_session *session,
 		seq->status_flags |= SEQ4_STATUS_ADMIN_STATE_REVOKED;
 }
 
+static bool nfsd4_slots_inuse(struct nfsd4_session *ses, int from)
+{
+	int i;
+
+	for (i = from; i < ses->se_fchannel.maxreqs; i++) {
+		struct nfsd4_slot *slot = xa_load(&ses->se_slots, i);
+
+		if (slot->sl_flags & NFSD4_SLOT_INUSE)
+			return true;
+	}
+	return false;
+}
+
 __be32
 nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		union nfsd4_op_u *u)
@@ -4575,7 +4588,9 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
 	if (session->se_target_maxslots < session->se_fchannel.maxreqs &&
 	    slot->sl_generation == session->se_slot_gen &&
-	    seq->maxslots <= session->se_target_maxslots)
+	    seq->maxslots <= session->se_target_maxslots &&
+	    seq->slotid < session->se_target_maxslots &&
+	    !nfsd4_slots_inuse(session, session->se_target_maxslots))
 		/* Client acknowledged our reduce maxreqs */
 		free_session_slots(session, session->se_target_maxslots);