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

[Jeff Layton]

On Thu, 2026-09-17 at 16:31 +0000, Wentao Liang wrote:
> idtoname_lookup() returns the id-to-name cache entry with a new
> reference. The name is parsed between the lookup and the update call,
> and when the parsed name is malformed (negative length or longer than
> IDMAP_NAMESZ), idtoname_parse() jumps to the error label without
> dropping the reference returned by idtoname_lookup(), leaking the cache
> entry.
> 
> Release the reference with cache_put() before taking the error exit.
> 
> Fixes: f9ecc921b5b5 ("[PATCH] knfsd: Use new cache code for name/id lookup caches")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Wentao Liang <vulab@xxxxxxxxxxx>
> ---
>  fs/nfsd/nfs4idmap.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
> index ba06d3d3e6dd..e0f1bebdf660 100644
> --- a/fs/nfsd/nfs4idmap.c
> +++ b/fs/nfsd/nfs4idmap.c
> @@ -252,8 +252,10 @@ idtoname_parse(struct cache_detail *cd, char *buf, int buflen)
>  	/* Name */
>  	error = -EINVAL;
>  	len = qword_get(&buf, buf1, PAGE_SIZE);
> -	if (len < 0 || len >= IDMAP_NAMESZ)
> +	if (len < 0 || len >= IDMAP_NAMESZ) {
> +		cache_put(&res->h, cd);
>  		goto out;
> +	}
>  	if (len == 0)
>  		set_bit(CACHE_NEGATIVE, &ent.h.flags);
>  	else

Looks like we have the same bug later if idtoname_update() fails too? I
think it would be better to restructure the code near the end of the
function for better error handling. Something like this maybe?

diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
index 4e5297593963..9fba65bb6ba4 100644
--- a/fs/nfsd/nfs4idmap.c
+++ b/fs/nfsd/nfs4idmap.c
@@ -255,7 +255,7 @@ idtoname_parse(struct cache_detail *cd, char *buf,
int buflen)
        error = -EINVAL;
        len = qword_get(&buf, buf1, PAGE_SIZE);
        if (len < 0 || len >= IDMAP_NAMESZ)
-               goto out;
+               goto out_put;
        if (len == 0)
                set_bit(CACHE_NEGATIVE, &ent.h.flags);
        else
@@ -263,10 +263,11 @@ idtoname_parse(struct cache_detail *cd, char
*buf, int buflen)
        error = -ENOMEM;
        res = idtoname_update(cd, &ent, res);
        if (res == NULL)
-               goto out;
+               goto out_put;
 
-       cache_put(&res->h, cd);
        error = 0;
+out_put:
+       cache_put(&res->h, cd);
 out:
        kfree(buf1);
        return error;