nfs4_client_to_reclaim() may return NULL if alloc_reclaim() fails. 
The caller __nfsd4_create_reclaim_record_grace() then unconditionally
dereferences the returned pointer via crp->cr_clp = clp, leading to a
null-ptr-deref crash.

Add a NULL check before assignment. If crp is NULL, just return.

Fixes: 4552f4e3f2c9 ("nfsd: change nfs4_client_to_reclaim() to allocate data")
Signed-off-by: Lai Zewei <laizewei3@huawei.com>
---
 fs/nfsd/nfs4recover.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
index 6ea25a52d2f4..005e3990da99 100644
--- a/fs/nfsd/nfs4recover.c
+++ b/fs/nfsd/nfs4recover.c
@@ -116,7 +116,8 @@ __nfsd4_create_reclaim_record_grace(struct nfs4_client *clp,
 	struct nfs4_client_reclaim *crp;
 
 	crp = nfs4_client_to_reclaim(name, princhash, nn);
-	crp->cr_clp = clp;
+	if (crp)
+		crp->cr_clp = clp;
 }
 
 static void
-- 
2.52.0

On Thu, Aug 6, 2026, at 3:37 AM, Lai Zewei wrote:
> nfs4_client_to_reclaim() may return NULL if alloc_reclaim() fails. 
> The caller __nfsd4_create_reclaim_record_grace() then unconditionally
> dereferences the returned pointer via crp->cr_clp = clp, leading to a
> null-ptr-deref crash.
>
> Add a NULL check before assignment. If crp is NULL, just return.
>
> Fixes: 4552f4e3f2c9 ("nfsd: change nfs4_client_to_reclaim() to allocate data")
> Signed-off-by: Lai Zewei <laizewei3@huawei.com>

First a process note: I don't see v1 through v4 on linux-nfs, and this
posting carries no changelog. If those were internal review rounds, you
should post the first public version as v1, or add a changelog saying
what changed between the previous revisions.

The code change is nominally correct. Commit 4552f4e3f2c9 moved the
allocations into nfs4_client_to_reclaim() and removed this caller's
NULL check along with the kfree() that check guarded. Restoring it
keeps this caller consistent with load_recdir() and
__cld_pipe_inprogress_downcall(), and both handle a NULL return.

However, IMO a UAF crash is not reachable. struct nfs4_client_reclaim
is just 48 bytes, and a GFP_KERNEL allocation of that size is almost
guaranteed to succeed 100% of the time on production systems. (I'll
redact the analysis of the allocator code path here).

The Fixes: tag then asks stable maintainers to backport a fix for it,
and I wonder if there's any need to.

Please send a v6 whose description says the check was dropped by
commit 4552f4e3f2c9 and is being restored, removes the Fixes: tag, and
states that the NULL return is not reachable from this caller. If the
defect was found using a static analysis tool or from slab fault
injection rather than observed in production, mention that in the
description too.

In addition, the subject names nfsd4_create_reclaim_record_grace, but
the function is __nfsd4_create_reclaim_record_grace; and "in
nfs4recover" repeats what the "nfsd:" prefix already says. Something
like "nfsd: restore nfs4_client_to_reclaim() error check" would be
more accurate.

Do let me know if I've missed something that makes this patch more
than a clean-up.


-- 
Chuck Lever

On Thu, 2026-08-06 at 15:37 +0800, Lai Zewei wrote:
> nfs4_client_to_reclaim() may return NULL if alloc_reclaim() fails. 
> The caller __nfsd4_create_reclaim_record_grace() then unconditionally
> dereferences the returned pointer via crp->cr_clp = clp, leading to a
> null-ptr-deref crash.
> 
> Add a NULL check before assignment. If crp is NULL, just return.
> 
> Fixes: 4552f4e3f2c9 ("nfsd: change nfs4_client_to_reclaim() to allocate data")
> Signed-off-by: Lai Zewei <laizewei3@huawei.com>
> ---
>  fs/nfsd/nfs4recover.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfsd/nfs4recover.c b/fs/nfsd/nfs4recover.c
> index 6ea25a52d2f4..005e3990da99 100644
> --- a/fs/nfsd/nfs4recover.c
> +++ b/fs/nfsd/nfs4recover.c
> @@ -116,7 +116,8 @@ __nfsd4_create_reclaim_record_grace(struct nfs4_client *clp,
>  	struct nfs4_client_reclaim *crp;
>  
>  	crp = nfs4_client_to_reclaim(name, princhash, nn);
> -	crp->cr_clp = clp;
> +	if (crp)
> +		crp->cr_clp = clp;
>  }
>  
>  static void

I suppose this is the best we can do here.

As an aside, it's probably time to start a discussion about (finally)
deprecating the legacy client tracking code. We've had it marked as
"legacy" for several years now, and the intention was always to
eventually get rid of it.

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