mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] net/9p/usbg: Fix use-after-free on usb9pfs_clear_tx
@ 2026-05-29  8:10 Yizhou Zhao
  2026-09-13  8:44 ` Dominique Martinet
  2026-09-15  6:51 ` Michael Grzeschik
  0 siblings, 2 replies; 5+ messages in thread
From: Yizhou Zhao @ 2026-05-29  8:10 UTC (permalink / raw)
  To: v9fs
  Cc: Yizhou Zhao, Eric Van Hensbergen, Latchesar Ionkov,
	Dominique Martinet, Christian Schoenebeck, linux-kernel,
	Yuxiang Yang, Ao Wang, Xuewei Feng, Qi Li, Ke Xu

When p9_usbg_close() is called, it invokes usb9pfs_clear_tx() but does
not clear usb9pfs->client nor usb9pfs->in_req->context afterwards.
The 9p client core (p9_client_destroy) calls close() and then
kfree(clnt), so after p9_usbg_close returns the client struct is freed.

If the USB gadget is later disabled (e.g., host disconnect),
usb9pfs_disable calls usb9pfs_clear_tx again, which passes the dangling
usb9pfs->client pointer to p9_client_cb. Inside p9_client_cb,
p9_req_put(c, req) dereferences the freed client to access its reqs IDR,
causing a use-after-free.

Additionally, usb9pfs_clear_tx does not clear usb9pfs->in_req->context
after calling p9_client_cb, so a second invocation finds a stale request
pointer and attempts double-completion, further compounding the UAF risk.

Fix by:
1. Clearing usb9pfs->in_req->context to NULL in usb9pfs_clear_tx()
   before checking usb9pfs->client, so that even if the client is gone
   the stale request pointer is consumed and cannot cause
   double-completion.
2. Adding a NULL check on usb9pfs->client in usb9pfs_clear_tx() to skip
   p9_client_cb() when the client has already been destroyed.
3. Setting usb9pfs->client = NULL in p9_usbg_close() after calling
   usb9pfs_clear_tx(), to prevent any subsequent call from dereferencing
   the freed client pointer.

Fixes: a3be076dc174 ("net/9p/usbg: Add new usb gadget function transport")
Reported-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
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:GLM-5.1
Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
---
diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
index 1ce7033..c75de5c 100644
--- a/net/9p/trans_usbg.c
+++ b/net/9p/trans_usbg.c
@@ -435,6 +435,11 @@ static void usb9pfs_clear_tx(struct f_usb9pfs *usb9pfs)
 	if (!req)
 		return;
 
+	usb9pfs->in_req->context = NULL;
+
+	if (!usb9pfs->client)
+		return;
+
 	if (!req->t_err)
 		req->t_err = -ECONNRESET;
 
@@ -457,6 +462,7 @@ static void p9_usbg_close(struct p9_client *client)
 	client->status = Disconnected;
 
 	usb9pfs_clear_tx(usb9pfs);
+	usb9pfs->client = NULL;
 
 	opts = container_of(usb9pfs->function.fi,
 			    struct f_usb9pfs_opts, func_inst);

--
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net/9p/usbg: Fix use-after-free on usb9pfs_clear_tx
  2026-05-29  8:10 [PATCH] net/9p/usbg: Fix use-after-free on usb9pfs_clear_tx Yizhou Zhao
@ 2026-09-13  8:44 ` Dominique Martinet
  2026-09-15  6:54   ` Michael Grzeschik
  2026-09-15  6:51 ` Michael Grzeschik
  1 sibling, 1 reply; 5+ messages in thread
From: Dominique Martinet @ 2026-09-13  8:44 UTC (permalink / raw)
  To: Michael Grzeschik
  Cc: v9fs, Eric Van Hensbergen, Latchesar Ionkov,
	Christian Schoenebeck, linux-kernel, Yuxiang Yang, Ao Wang,
	Xuewei Feng, Qi Li, Ke Xu, Yizhou Zhao

Hi Michael,

I'm honestly not quite sure what to do with this and others UAF usb9pfs
patches -- you acked the first one I replied to, and the tag match is
obviously correct so I picked these two, but I'd like your opinion on
the rest for two reasons:
 - There seem to be plenty of rough edges around usb9pfs, and frankly I
don't consider this a trust boundary (the usecase being developping
embedded devices, you trust the server fully, so whatever happens
in corner cases doesn't matter)
 - You had a rework in progress (before leaving Pengutronix, which I can
 totally understand you don't have time or want to keep carrying going
 forward, but is someone else from Pengutronix taking your place?)

For all I know this is also correct, but I'm not sure slapping a bandaid
is the way to go, so I'd really appreciate if you could have a look at
this[1] and "net/9p/usbg: clear stale request context after abort"[2]
[1] https://lore.kernel.org/r/20260529081026.77732-1-zhaoyz24@mails.tsinghua.edu.cn
[2] https://lore.kernel.org/r/20260802152148.810390-1-marsy12010123@gmail.com

If you tell me to pick them up I will, and if I don't hear back from you
I'll take another look hopefully before the 7.4 deadline, but I honestly
don't have enough time for this so the most likely outcome will be
/dev/null

Thanks (and sorry Yizhou Zhao / Chengfeng Ye)
-- 
Dominique

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net/9p/usbg: Fix use-after-free on usb9pfs_clear_tx
  2026-05-29  8:10 [PATCH] net/9p/usbg: Fix use-after-free on usb9pfs_clear_tx Yizhou Zhao
  2026-09-13  8:44 ` Dominique Martinet
@ 2026-09-15  6:51 ` Michael Grzeschik
  1 sibling, 0 replies; 5+ messages in thread
From: Michael Grzeschik @ 2026-09-15  6:51 UTC (permalink / raw)
  To: Yizhou Zhao
  Cc: v9fs, Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
	Christian Schoenebeck, linux-kernel, Yuxiang Yang, Ao Wang,
	Xuewei Feng, Qi Li, Ke Xu

On Fri, May 29, 2026 at 04:10:25PM +0800, Yizhou Zhao wrote:
> When p9_usbg_close() is called, it invokes usb9pfs_clear_tx() but does
> not clear usb9pfs->client nor usb9pfs->in_req->context afterwards.
> The 9p client core (p9_client_destroy) calls close() and then
> kfree(clnt), so after p9_usbg_close returns the client struct is freed.
> 
> If the USB gadget is later disabled (e.g., host disconnect),
> usb9pfs_disable calls usb9pfs_clear_tx again, which passes the dangling
> usb9pfs->client pointer to p9_client_cb. Inside p9_client_cb,
> p9_req_put(c, req) dereferences the freed client to access its reqs IDR,
> causing a use-after-free.
> 
> Additionally, usb9pfs_clear_tx does not clear usb9pfs->in_req->context
> after calling p9_client_cb, so a second invocation finds a stale request
> pointer and attempts double-completion, further compounding the UAF risk.
> 
> Fix by:
> 1. Clearing usb9pfs->in_req->context to NULL in usb9pfs_clear_tx()
>    before checking usb9pfs->client, so that even if the client is gone
>    the stale request pointer is consumed and cannot cause
>    double-completion.
> 2. Adding a NULL check on usb9pfs->client in usb9pfs_clear_tx() to skip
>    p9_client_cb() when the client has already been destroyed.
> 3. Setting usb9pfs->client = NULL in p9_usbg_close() after calling
>    usb9pfs_clear_tx(), to prevent any subsequent call from dereferencing
>    the freed client pointer.
> 
> Fixes: a3be076dc174 ("net/9p/usbg: Add new usb gadget function transport")
> Reported-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
> 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:GLM-5.1
> Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>

Acked-by: Michael Grzeschik <mgr@kernel.org>

> ---
> diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
> index 1ce7033..c75de5c 100644
> --- a/net/9p/trans_usbg.c
> +++ b/net/9p/trans_usbg.c
> @@ -435,6 +435,11 @@ static void usb9pfs_clear_tx(struct f_usb9pfs *usb9pfs)
>  	if (!req)
>  		return;
>  
> +	usb9pfs->in_req->context = NULL;
> +
> +	if (!usb9pfs->client)
> +		return;
> +
>  	if (!req->t_err)
>  		req->t_err = -ECONNRESET;
>  
> @@ -457,6 +462,7 @@ static void p9_usbg_close(struct p9_client *client)
>  	client->status = Disconnected;
>  
>  	usb9pfs_clear_tx(usb9pfs);
> +	usb9pfs->client = NULL;
>  
>  	opts = container_of(usb9pfs->function.fi,
>  			    struct f_usb9pfs_opts, func_inst);
> 
> --
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net/9p/usbg: Fix use-after-free on usb9pfs_clear_tx
  2026-09-13  8:44 ` Dominique Martinet
@ 2026-09-15  6:54   ` Michael Grzeschik
  2026-09-15  7:09     ` Dominique Martinet
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Grzeschik @ 2026-09-15  6:54 UTC (permalink / raw)
  To: Dominique Martinet
  Cc: v9fs, Eric Van Hensbergen, Latchesar Ionkov,
	Christian Schoenebeck, linux-kernel, Yuxiang Yang, Ao Wang,
	Xuewei Feng, Qi Li, Ke Xu, Yizhou Zhao

Hi Dominique

On Sun, Sep 13, 2026 at 05:44:34PM +0900, Dominique Martinet wrote:
> I'm honestly not quite sure what to do with this and others UAF usb9pfs
> patches -- you acked the first one I replied to, and the tag match is
> obviously correct so I picked these two, but I'd like your opinion on
> the rest for two reasons:
>  - There seem to be plenty of rough edges around usb9pfs, and frankly I
> don't consider this a trust boundary (the usecase being developping
> embedded devices, you trust the server fully, so whatever happens
> in corner cases doesn't matter)

I agree that the edges are there and need to be fixed.

>  - You had a rework in progress (before leaving Pengutronix, which I can
>  totally understand you don't have time or want to keep carrying going
>  forward, but is someone else from Pengutronix taking your place?)

I am actually carrying to improve the series. I just now got some time
and started to build an setup where I can easily test further changes.

Also my leftover patches are also tested and ready for updates on the
list.

> For all I know this is also correct, but I'm not sure slapping a bandaid
> is the way to go, so I'd really appreciate if you could have a look at
> this[1] and "net/9p/usbg: clear stale request context after abort"[2]
> [1] https://lore.kernel.org/r/20260529081026.77732-1-zhaoyz24@mails.tsinghua.edu.cn
> [2] https://lore.kernel.org/r/20260802152148.810390-1-marsy12010123@gmail.com
> 
> If you tell me to pick them up I will, and if I don't hear back from you
> I'll take another look hopefully before the 7.4 deadline, but I honestly
> don't have enough time for this so the most likely outcome will be
> /dev/null

I just acked the two and rebased my further work on them.

> Thanks (and sorry Yizhou Zhao / Chengfeng Ye)

Thank you for caring and sorry for the long delay. Things are in
steady motion, but will hopefully smooth out soon. ;)

> -- 
> Dominique

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] net/9p/usbg: Fix use-after-free on usb9pfs_clear_tx
  2026-09-15  6:54   ` Michael Grzeschik
@ 2026-09-15  7:09     ` Dominique Martinet
  0 siblings, 0 replies; 5+ messages in thread
From: Dominique Martinet @ 2026-09-15  7:09 UTC (permalink / raw)
  To: Michael Grzeschik
  Cc: v9fs, Eric Van Hensbergen, Latchesar Ionkov,
	Christian Schoenebeck, linux-kernel, Yuxiang Yang, Ao Wang,
	Xuewei Feng, Qi Li, Ke Xu, Yizhou Zhao

Thank you for the acks!

Michael Grzeschik wrote on Tue, Sep 15, 2026 at 08:54:22AM +0200:
> >  - You had a rework in progress (before leaving Pengutronix, which I can
> >  totally understand you don't have time or want to keep carrying going
> >  forward, but is someone else from Pengutronix taking your place?)
> 
> I am actually carrying to improve the series. I just now got some time
> and started to build an setup where I can easily test further changes.

That is great news, thank you!

> Also my leftover patches are also tested and ready for updates on the
> list.

I'm afraid I've lost track of them, I could dig them out but am more
likely to fudge something up than not, so could you resend after
rebasing on top of https://github.com/martinetd/linux/commits/9p-test ?
(that should contain all of the pending patches I picked up so far,
including the two you just looked at; I'll add your Acks later™)

> > Thanks (and sorry Yizhou Zhao / Chengfeng Ye)
> 
> Thank you for caring and sorry for the long delay. Things are in
> steady motion, but will hopefully smooth out soon. ;)

No worry, I've been MIA a few months as well - I honestly didn't expect
you to continue (seriously) so this is awesome news :)

Cheers,
-- 
Dominique Martinet | Asmadeus

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-15  7:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-29  8:10 [PATCH] net/9p/usbg: Fix use-after-free on usb9pfs_clear_tx Yizhou Zhao
2026-09-13  8:44 ` Dominique Martinet
2026-09-15  6:54   ` Michael Grzeschik
2026-09-15  7:09     ` Dominique Martinet
2026-09-15  6:51 ` Michael Grzeschik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®