Re: [PATCH] nfsd: Fix id-to-name cache entry leak in idtoname_parse()

[Jeff Layton]

On Thu, 2026-09-17 at 14:53 -0400, Chuck Lever wrote:
> On Thu, 2026-09-17 at 14:10 -0400, Jeff Layton wrote:
> > Looks like we have the same bug later if idtoname_update() fails too?
> 
> I don't think so: when sunrpc_cache_update() fails to allocate the
> replacement entry, it drops the reference on the old entry itself:
> 
> 	tmp = detail->alloc();
> 	if (!tmp) {
> 		cache_put(old, detail);
> 		return NULL;
> 	}
> 
> So that exit is already balanced.
> 
> > +out_put:
> > +       cache_put(&res->h, cd);
> >  out:
> 
> On the update failure path, res is NULL here, and cache_put()
> dereferences it. A shared label would need the "if (res)" guard that
> expkey_parse() uses.
> 
> A shared put label might be a reasonable clean-up, but for a fix
> headed to stable, the two-line version Wentao posted looks OK
> to me.
> 

I stand corrected then re: the other bug. It might be nice to clean
that up and make the refcounting more evident, but I agree that should
wait for a later patch.

You can add this:

Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>