Re: [PATCH] 9p: avoid putting oldfid in p9_client_walk() error path

Dominique Martinet <[email protected]> Fri, 29 May 2026 10:50:59 +0900
Newsgroups dev.linux.lists.v9fs,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Message-ID <[email protected]>
Yizhou Zhao wrote on Thu, May 28, 2026 at 01:39:16PM +0800:
> When p9_client_walk() is called with clone set to false, fid aliases
> oldfid. If the walk subsequently fails after the request has been sent,
> the error path jumps to clunk_fid, which currently calls p9_fid_put(fid)
> unconditionally.
> 
> This drops a reference to oldfid even though ownership of oldfid remains
> with the caller. If this is the last reference, oldfid can be clunked and
> destroyed while the caller still expects it to be valid. A later use or
> put of oldfid can then trigger a use-after-free or refcount underflow.
> 
> Fix this by only putting fid in the clunk_fid error path when it does not
> alias oldfid, matching the existing guard in the error path below.
> 
> This can be triggered when a multi-component walk is split into multiple
> p9_client_walk() calls and a later non-cloning walk fails. A reproducer
> and refcount warning logs are available on request.
> 
> Fixes: b48dbb998d70 ("9p fid refcount: add p9_fid_get/put wrappers")
> Cc: [email protected]
> Reported-by: Yuxiang Yang <[email protected]>
> Reported-by: Ao Wang <[email protected]>
> Reported-by: Xuewei Feng <[email protected]>
> Reported-by: Qi Li <[email protected]>
> Reported-by: Ke Xu <[email protected]>
> Assisted-by: GLM 5.1
> Signed-off-by: Yizhou Zhao <[email protected]>

This makes sense, thanks.
Queueing the patch.

> ---
>  net/9p/client.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/9p/client.c b/net/9p/client.c
> index f0dcf25..4b942d0 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -1092,7 +1092,8 @@ struct p9_fid *p9_client_walk(struct p9_fid *oldfid, uint16_t nwname,
>  
>  clunk_fid:
>  	kfree(wqids);
> -	p9_fid_put(fid);
> +	if (fid != oldfid)
> +		p9_fid_put(fid);
>  	fid = NULL;
>  
>  error:

-- 
Dominique Martinet | Asmadeus