[PATCH] nfsd: fix use-after-free in nfsd4_lock() when releasing new lock stateid

[PATCH] nfsd: fix use-after-free in nfsd4_lock() when releasing new lock stateid

[Wentao Liang]

When processing a new lock request that fails with an error,
nfsd4_lock() may call release_lock_stateid() on a newly created
lock stateid.  release_lock_stateid() can free the stateid via
nfs4_put_stid() if it is the last reference, but the caller
subsequently accesses lock_stp->st_mutex and lock_stp->st_stid,
leading to a use-after-free.

Fix this by moving mutex_unlock() before release_lock_stateid()
and jumping over the remaining lock_stp cleanup after release,
since release_lock_stateid() already handles the final put.
This ensures no further access to lock_stp after it may be freed.

Fixes: 5db1c03feb00 ("nfsd: clean up lockowner refcounting when finding them")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 fs/nfsd/nfs4state.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 88c347957da5..cef9d8ddfc43 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -8388,13 +8388,18 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		 * If this is a new, never-before-used stateid, and we are
 		 * returning an error, then just go ahead and release it.
 		 */
-		if (status && new)
+		if (status && new) {
+			mutex_unlock(&lock_stp->st_mutex);
 			release_lock_stateid(lock_stp);
+			goto out_no_lock_stp;
+		}
 
 		mutex_unlock(&lock_stp->st_mutex);
 
 		nfs4_put_stid(&lock_stp->st_stid);
 	}
+
+out_no_lock_stp:
 	if (open_stp)
 		nfs4_put_stid(&open_stp->st_stid);
 	nfsd4_bump_seqid(cstate, status);
-- 
2.34.1

Re: [PATCH] nfsd: fix use-after-free in nfsd4_lock() when releasing new lock stateid

[Jeff Layton]

On Mon, 2026-05-18 at 15:30 +0000, Wentao Liang wrote:
> When processing a new lock request that fails with an error,
> nfsd4_lock() may call release_lock_stateid() on a newly created
> lock stateid.  release_lock_stateid() can free the stateid via
> nfs4_put_stid() if it is the last reference, but the caller
> subsequently accesses lock_stp->st_mutex and lock_stp->st_stid,
> leading to a use-after-free.
> 
> Fix this by moving mutex_unlock() before release_lock_stateid()
> and jumping over the remaining lock_stp cleanup after release,
> since release_lock_stateid() already handles the final put.
> This ensures no further access to lock_stp after it may be freed.
> 
> Fixes: 5db1c03feb00 ("nfsd: clean up lockowner refcounting when finding them")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>  fs/nfsd/nfs4state.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 88c347957da5..cef9d8ddfc43 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -8388,13 +8388,18 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  		 * If this is a new, never-before-used stateid, and we are
>  		 * returning an error, then just go ahead and release it.
>  		 */
> -		if (status && new)
> +		if (status && new) {
> +			mutex_unlock(&lock_stp->st_mutex);
>  			release_lock_stateid(lock_stp);
> +			goto out_no_lock_stp;
> +		}
>  
>  		mutex_unlock(&lock_stp->st_mutex);
>  
>  		nfs4_put_stid(&lock_stp->st_stid);
>  	}
> +
> +out_no_lock_stp:
>  	if (open_stp)
>  		nfs4_put_stid(&open_stp->st_stid);
>  	nfsd4_bump_seqid(cstate, status);

This patch is not correct. It's not terribly obvious, but at the point
of the start of the delta, lock_stp has a refcount of 2, so this
problem can't happen:

  1. nfs4_alloc_stid() — sets sc_count = 1
  2. init_lock_stateid() line 8193 — refcount_inc(&stp->st_stid.sc_count) → sc_count = 2

I don't think there is a bug here.
-- 
Jeff Layton <jlayton@kernel.org>