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

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

[Yizhou Zhao]

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: stable@vger.kernel.org
Reported-by: Yuxiang Yang <yangyx22@mails.tsinghua.edu.cn>
Reported-by: Ao Wang <wangao@seu.edu.cn>
Reported-by: Xuewei Feng <fengxw06@126.com>
Reported-by: Qi Li <qli01@tsinghua.edu.cn>
Reported-by: Ke Xu <xuke@tsinghua.edu.cn>
Assisted-by: GLM 5.1
Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
---
 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:
-- 
2.43.0

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

[Dominique Martinet]

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: stable@vger.kernel.org
> Reported-by: Yuxiang Yang <yangyx22@mails.tsinghua.edu.cn>
> Reported-by: Ao Wang <wangao@seu.edu.cn>
> Reported-by: Xuewei Feng <fengxw06@126.com>
> Reported-by: Qi Li <qli01@tsinghua.edu.cn>
> Reported-by: Ke Xu <xuke@tsinghua.edu.cn>
> Assisted-by: GLM 5.1
> Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>

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