Re: [PATCH v2] nfsd: pin OPEN stateid referenced by LOCK stateid

[Jeff Layton <jlayton@kernel.org>]

On Wed, 2026-09-30 at 14:36 +0900, Hyunsol Mun wrote:
> A LOCK stateid stores its parent OPEN stateid in st_openstp without taking
> a reference. A request can keep the child alive while a concurrent CLOSE
> unhashes the child and releases the parent's persistent reference. A READ
> using the surviving LOCK stateid then follows the freed parent in
> nfs4_check_openmode().
> 
> Take an OPEN stateid reference when initializing the LOCK stateid and
> release it from the LOCK stateid free callback. Keep the free path safe
> for partially initialized objects.
> 
> An unprivileged NFSv4.1 client with normal access to an exported file can
> reach the lifetime error using ordinary OPEN, LOCK, READ, and CLOSE
> operations. Validation used the nfsd-testing base recorded below. Natural
> race runs on the unmodified KASAN kernel completed 3,000 one-reader
> attempts and 5,000 eight-reader attempts without reproducing the report.
> 
> For deterministic validation, a test-only kprobe delayed
> nfs4_check_openmode() after LOCK setup, before it followed st_openstp. The
> module only widened the race window; it did not allocate, free, or modify
> an NFSD object. The unpatched kernel then reported a slab use-after-free in
> nfs4_check_openmode() on the first timed attempt. With this patch, the same
> timed READ and CLOSE both returned NFS4_OK and produced no KASAN report.
> 
> The patched kernel also completed the natural 3,000-attempt run and the
> 5,000-attempt eight-reader run without a KASAN report or kernel failure.
> The full KASAN kernel built with CONFIG_WERROR without a compiler
> diagnostic. Basic NFSv4.2 and NFSv3 read/write/unmount smoke tests passed.
> A source reproducer, timing-module source, complete logs, and the kernel
> configuration are available privately on request.
> 
> The vulnerability research and validation were conducted by members of
> the Tobabz team as part of the Best of the Best 15th program.
> 
> Fixes: 02921914170e ("nfsd4: fix openmode checking on IO using lock stateid")
> Assisted-by: LLM
> Signed-off-by: Hyunsol Mun <muumthf@gmail.com>
> ---
> Changes in v2:
> - Separate natural-race and timing-assisted validation results.
> - Add full-build and NFSv4.2/NFSv3 smoke-test results.
> - No code changes.
> 
>  fs/nfsd/nfs4state.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index bbc16dd22..208badfe6 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1781,6 +1781,7 @@ static void nfs4_free_ol_stateid(struct nfs4_stid *stid)
>  static void nfs4_free_lock_stateid(struct nfs4_stid *stid)
>  {
>  	struct nfs4_ol_stateid *stp = openlockstateid(stid);
> +	struct nfs4_ol_stateid *open_stp = stp->st_openstp;
>  	struct nfs4_lockowner *lo = lockowner(stp->st_stateowner);
>  	struct nfsd_file *nf;
>  
> @@ -1791,6 +1792,8 @@ static void nfs4_free_lock_stateid(struct nfs4_stid *stid)
>  		nfsd_file_put(nf);
>  	}
>  	nfs4_free_ol_stateid(stid);
> +	if (open_stp)
> +		nfs4_put_stid(&open_stp->st_stid);
>  }
>  
>  /*
> @@ -9301,6 +9304,7 @@ init_lock_stateid(struct nfs4_ol_stateid *stp, struct nfs4_lockowner *lo,
>  			exp_get(open_stp->st_stid.sc_export);
>  	stp->st_access_bmap = 0;
>  	stp->st_deny_bmap = open_stp->st_deny_bmap;
> +	refcount_inc(&open_stp->st_stid.sc_count);
>  	stp->st_openstp = open_stp;
>  	spin_lock(&fp->fi_lock);
>  	list_add(&stp->st_locks, &open_stp->st_locks);
> 
> base-commit: 32eb1a60b456980761cf7a9cee8f907fdc08afb8

LGTM

Reviewed-by: Jeff Layton <jlayton@kernel.org>