[PATCH 1/2] NFS: Pin the 'struct nfs_server' during a FREE_STATEID call

[PATCH 1/2] NFS: Pin the 'struct nfs_server' during a FREE_STATEID call

[Anna Schumaker]

From: Anna Schumaker <anna.schumaker@hammerspace.com>

Dan Aloni reports that he was able to hit a use-after-free bug if a
FREE_STATEID operation gets delayed for whatever reason. Fix this by
bumping the refcount of the 'struct nfs_server' object for the duration
of the FREE_STATEID so it doesn't get cleaned up from underneath us
while operations are still in flight.

Reported-by: Dan Aloni <dan.aloni@vastdata.com>
Fixes: 7c1d5fae4a87 ("NFSv4: Convert nfs41_free_stateid to use an asynchronous RPC call")
Tested-by: Dan Aloni <dan.aloni@vastdata.com>
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
---
 fs/nfs/nfs4proc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index a3415082d610..3ad5ef52a2e8 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -10364,6 +10364,7 @@ static void nfs41_free_stateid_release(void *calldata)
 	struct nfs_free_stateid_data *data = calldata;
 	struct nfs_client *clp = data->server->nfs_client;
 
+	nfs_sb_deactive(data->server->super);
 	nfs_put_client(clp);
 	kfree(calldata);
 }
@@ -10405,6 +10406,10 @@ static int nfs41_free_stateid(struct nfs_server *server,
 
 	if (!refcount_inc_not_zero(&clp->cl_count))
 		return -EIO;
+	if (!nfs_sb_active(server->super)) {
+		nfs_put_client(clp);
+		return -EIO;
+	}
 
 	nfs4_state_protect(clp, NFS_SP4_MACH_CRED_STATEID,
 		&task_setup.rpc_client, &msg);
-- 
2.55.0

[PATCH 2/2] NFS: Decrement refcounts if allocating nfs_free_stateid_data fails

[Anna Schumaker]

From: Anna Schumaker <anna.schumaker@hammerspace.com>

I noticed that we were immediately exiting this function if the
allocation fails, leaving the client and server object refcounts bumped.
Fix this by creating a common exit point to clean up dangling
references.

Fixes: 576acc259146 ("nfs4: take a reference on the nfs_client when running FREE_STATEID")
Signed-off-by: Anna Schumaker <anna.schumaker@hammerspace.com>
---
 fs/nfs/nfs4proc.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 3ad5ef52a2e8..5709c6fea85b 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -10403,21 +10403,22 @@ static int nfs41_free_stateid(struct nfs_server *server,
 	struct nfs_free_stateid_data *data;
 	struct rpc_task *task;
 	struct nfs_client *clp = server->nfs_client;
+	int ret = -EIO;
 
 	if (!refcount_inc_not_zero(&clp->cl_count))
-		return -EIO;
-	if (!nfs_sb_active(server->super)) {
-		nfs_put_client(clp);
-		return -EIO;
-	}
+		return ret;
+	if (!nfs_sb_active(server->super))
+		goto out_put_clp;
 
 	nfs4_state_protect(clp, NFS_SP4_MACH_CRED_STATEID,
 		&task_setup.rpc_client, &msg);
 
 	dprintk("NFS call  free_stateid %p\n", stateid);
 	data = kmalloc_obj(*data);
-	if (!data)
-		return -ENOMEM;
+	if (!data) {
+		ret = -ENOMEM;
+		goto out_put_server;
+	}
 	data->server = server;
 	nfs4_stateid_copy(&data->args.stateid, stateid);
 
@@ -10433,6 +10434,11 @@ static int nfs41_free_stateid(struct nfs_server *server,
 	rpc_put_task(task);
 	stateid->type = NFS4_FREED_STATEID_TYPE;
 	return 0;
+out_put_server:
+	nfs_sb_deactive(server->super);
+out_put_clp:
+	nfs_put_client(clp);
+	return ret;
 }
 
 static void
-- 
2.55.0

Re: [PATCH 1/2] NFS: Pin the 'struct nfs_server' during a FREE_STATEID call

[Jia Zhu]

Hi Anna,

On Fri, Jul 10, 2026 at 08:47:14PM +0000, Anna Schumaker wrote:
> Dan Aloni reports that he was able to hit a use-after-free bug if a
> FREE_STATEID operation gets delayed for whatever reason. Fix this by
> bumping the refcount of the 'struct nfs_server' object for the duration
> of the FREE_STATEID so it doesn't get cleaned up from underneath us
> while operations are still in flight.

This patch reminded me of a similar nfs_server use-after-free that we
recently encountered in production. However, it appears to involve a
different race and root cause:

  NFSv4: pin the superblock for active state owners
  https://lore.kernel.org/all/20260707133623.23078-1-zhujia.zj@bytedance.com/

Would you mind taking a look at it and letting me know if the approach
looks reasonable?

Thanks,
Jia