Re: [PATCH] nfsd: don't free session slots that are still in use

[Chuck Lever]

On Tue, May 26, 2026, at 10:22 AM, Jeff Layton wrote:
> 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.
>
> This is a remotely triggerable heap use-after-free from any
> authenticated NFSv4.1+ client with an established session, requiring
> only that the kernel memory shrinker has run against nfsd session slots
> (a normal occurrence under memory pressure).
>
> Fix this by adding a check that the current request's slotid is below
> the shrink boundary before allowing the slot reduction to proceed.
>
> Fixes: 8fb77d12c76e ("nfsd: add support for freeing unused session-DRC slots")
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
>  fs/nfsd/nfs4state.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index d5cbf626ab9b..f90ad8281a95 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4848,7 +4848,8 @@ 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)
>  		/* Client acknowledged our reduce maxreqs */
>  		free_session_slots(session, session->se_target_maxslots);
> 

The patch guards only against the caller freeing its own slot. It
doesn't guard against freeing a different slot that is concurrently
in use by another thread. Perhaps free_session_slots() should skip
any slot with NFSD4_SLOT_INUSE set. Or the shrink could be deferred
until no high-numbered slots are in use?


-- 
Chuck Lever