Re: [PATCH v4 9/9] NFSD: Release the export reference when reaping open stateids

["Chuck Lever" <cel@kernel.org>]

On Thu, Jul 9, 2026, at 2:40 PM, Jeff Layton wrote:
> On Thu, 2026-07-09 at 13:40 -0400, Chuck Lever wrote:
>> nfs4_put_stid() releases the svc_export tracked in
>> nfs4_stid.sc_export, but free_ol_stateid_reaplist() frees open and
>> lock stateids by calling ->sc_free() directly, bypassing that path.
>> An open stateid takes an sc_export reference in nfs4_open() and a
>> lock stateid takes its own in init_lock_stateid(); both reach
>> free_ol_stateid_reaplist() through their normal teardown, the open
>> stateid via release_open_stateid() and the lock stateid via
>> nfsd4_release_lockowner(), each through put_ol_stateid_locked().
>> The reference is therefore never dropped, pinning the export and
>> blocking unmount for the lifetime of the stateid.
>> 
>> Release sc_export in free_ol_stateid_reaplist() the way
>> nfs4_put_stid() does. ->sc_free() runs once per stateid, and a
>> stateid reaches free_ol_stateid_reaplist() or nfs4_put_stid() but
>> never both, so the reference is dropped exactly once. Revoked
>> stateids reach this path with sc_export already cleared by
>> drop_stid_export(), so they are skipped rather than double-freed.
>> 
>> nfs4_put_stid() itself read sc_export before acquiring cl_lock.
>> drop_stid_export() clears that field and releases the reference
>> under cl_lock, so a concurrent revocation could drop the export in
>> the window between the read and the final put, releasing the same
>> reference twice. Read sc_export while cl_lock is held so the two
>> paths serialize and the reference is released exactly once.
>> 
>> Fixes: ba0cde5dc81d ("NFSD: Track svc_export in nfs4_stid")
>> Reported-by: sashiko-bot <sashiko-bot@kernel.org>
>> Closes: https://sashiko.dev/#/patchset/20260707-cel-v3-0-7c0cc16fd54f@kernel.org?part=9
>> Signed-off-by: Chuck Lever <cel@kernel.org>
>> ---
>>  fs/nfsd/nfs4state.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index 20556b8f186a..e988dfebf75e 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c

>> @@ -1753,9 +1756,12 @@ free_ol_stateid_reaplist(struct list_head *reaplist)
>>  				       st_locks);
>>  		list_del(&stp->st_locks);
>>  		fp = stp->st_stid.sc_file;
>> +		exp = stp->st_stid.sc_export;
>>  		stp->st_stid.sc_free(&stp->st_stid);
>>  		if (fp)
>>  			put_nfs4_file(fp);
>> +		if (exp)
>> +			exp_put(exp);
>
> nit: nfs4_put_stid() does this in the reverse order. It doesn't
> actually matter, but it looks a bit weird if you want to fix it up
> before merging.
>
>>  	}
>>  }
>>  
>
> Reviewed-by: Jeff Layton <jlayton@kernel.org>

Swap applied, series pushed to nfsd-testing. Thanks for your review!


-- 
Chuck Lever