Re: [PATCH] 9p: avoid putting oldfid in p9_client_walk() error path
From: Dominique Martinet
Date: Thu May 28 2026 - 21:56:38 EST
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@xxxxxxxxxxxxxxx
> Reported-by: Yuxiang Yang <yangyx22@xxxxxxxxxxxxxxxxxxxxx>
> Reported-by: Ao Wang <wangao@xxxxxxxxxx>
> Reported-by: Xuewei Feng <fengxw06@xxxxxxx>
> Reported-by: Qi Li <qli01@xxxxxxxxxxxxxxx>
> Reported-by: Ke Xu <xuke@xxxxxxxxxxxxxxx>
> Assisted-by: GLM 5.1
> Signed-off-by: Yizhou Zhao <zhaoyz24@xxxxxxxxxxxxxxxxxxxxx>
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