[PATCH] net/9p/usbg: Fix use-after-free in disable_usb9pfs()

[PATCH] net/9p/usbg: Fix use-after-free in disable_usb9pfs()

[Yizhou Zhao]

disable_usb9pfs() frees the IN and OUT usb_request objects before it
disables the corresponding endpoints. If either request is still queued,
the later usb_ep_disable() call cancels the endpoint queue and the UDC
driver can still access the already freed request.

With dummy_hcd and KASAN, this is reproducible by queueing the OUT
request and then disconnecting the configfs gadget:

  BUG: KASAN: slab-use-after-free in dummy_disable+0x2b4/0x300
  Read of size 8 at addr ffff888009702400 by task sh/1
  usb_ep_disable+0x8e/0x1f0
  usb9pfs_func_unbind+0x193/0x350
  gadget_dev_desc_UDC_store+0x135/0x280

dummy_free_request() also warns because the request is freed while its
queue entry is still linked.

Disable both endpoints before freeing the request objects. This lets
usb_ep_disable() cancel any queued transfers and invoke the completion
callback while the request storage is still valid. The request objects
are then freed only after they have been removed from the endpoint
queues.

Fixes: a3be076dc174 ("net/9p/usbg: Add new usb gadget function transport")
Cc: stable@vger.kernel.org
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>
---
 net/9p/trans_usbg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
index 1ce70338999c..5d0d6add150e 100644
--- a/net/9p/trans_usbg.c
+++ b/net/9p/trans_usbg.c
@@ -278,6 +278,9 @@ static void disable_usb9pfs(struct f_usb9pfs *usb9pfs)
 	struct usb_composite_dev *cdev =
 		usb9pfs->function.config->cdev;
 
+	disable_ep(cdev, usb9pfs->in_ep);
+	disable_ep(cdev, usb9pfs->out_ep);
+
 	if (usb9pfs->in_req) {
 		usb_ep_free_request(usb9pfs->in_ep, usb9pfs->in_req);
 		usb9pfs->in_req = NULL;
@@ -287,9 +290,6 @@ static void disable_usb9pfs(struct f_usb9pfs *usb9pfs)
 		usb_ep_free_request(usb9pfs->out_ep, usb9pfs->out_req);
 		usb9pfs->out_req = NULL;
 	}
-
-	disable_ep(cdev, usb9pfs->in_ep);
-	disable_ep(cdev, usb9pfs->out_ep);
 	dev_dbg(&cdev->gadget->dev, "%s disabled\n",
 		usb9pfs->function.name);
 }
-- 
2.43.0

Re: [PATCH] net/9p/usbg: Fix use-after-free in disable_usb9pfs()

[XIAO WU]

Hi Yizhou,

On Sun, Jun 07, 2026 at 09:01:16PM +0800, Yizhou Zhao wrote:
 > disable_usb9pfs() frees the IN and OUT usb_request objects before it
 > disables the corresponding endpoints. If either request is still queued,
 > the later usb_ep_disable() call cancels the endpoint queue and the UDC
 > driver can still access the already freed request.

This patch correctly moves disable_ep() before usb_ep_free_request() to
prevent the use-after-free in the endpoint cancellation path.

However, while verifying this patch with KASAN and dummy_hcd, I found a
separate bug in the alloc_requests() error path that still leads to a
kernel panic in usb9pfs_clear_tx() during gadget unbind.

The root cause is that alloc_requests() frees in_req on failure but
does not set usb9pfs->in_req to NULL, leaving a dangling pointer.
Later, when the gadget is unbound via configfs, the call chain
reset_config() -> usb9pfs_clear_tx() dereferences the dangling
in_req->context and crashes:

   Oops: general protection fault, probably for non-canonical address
     0xdffffc0000000060: 0000 [#1] SMP KASAN NOPTI
   KASAN: null-ptr-deref in range [0x0000000000000300-0x0000000000000307]
   CPU: 0 UID: 0 PID: 10145 Comm: rm Not tainted 7.1.0-rc6 #1
   RIP: 0010:strcmp+0x5b/0xb0
   Call Trace:
    <TASK>
    look_up_lock_class+0x6b/0x130
    register_lock_class+0x2cb/0x540
    __lock_acquire+0xac/0x2730
    lock_acquire+0x1ae/0x360
    __wake_up+0x21/0x60
    p9_client_cb+0x59/0x80
    usb9pfs_clear_tx+0xe1/0x150      <-- dereferences dangling 
in_req->context
    reset_config+0xbe/0x2b0
    __composite_disconnect+0xb6/0x160
    configfs_composite_disconnect+0xed/0x130
    usb_gadget_disconnect_locked+0x214/0x500
    gadget_unbind_driver+0xe2/0x520
    ...
    configfs_unlink+0x3f6/0x840
    vfs_unlink+0x2f5/0xbd0
    Kernel panic - not syncing: Fatal exception

The reproducer:

   1. Create a USB gadget with the usb9pfs function
   2. Set buflen=0 so that alloc_ep_req() fails inside alloc_requests()
   3. Link the function and enable the UDC (enable_usb9pfs() fails)
   4. Unbind the gadget (configfs unlink or echo "" > UDC)

I wrote the following PoC to trigger this bug.  It creates a USB
gadget with a usb9pfs function, sets buflen=0 so that alloc_ep_req()
fails in alloc_requests(), which frees in_req without NULLing the
pointer, then unbinds the gadget to trigger usb9pfs_clear_tx() on the
dangling in_req.

---8<--- poc.c ---
/*
  * PoC: Dangling pointer dereference in usb9pfs_clear_tx()
  *      via alloc_requests() failure path.
  *
  * Patch: net/9p/usbg: Fix use-after-free in disable_usb9pfs()
  *
  * alloc_requests()'s fail_in path frees usb9pfs->in_req without
  * NULLing the pointer.  Later, when the gadget is unbound,
  * usb9pfs_clear_tx() dereferences the dangling pointer.
  *
  * Trigger:
  *   1. Create USB gadget with usb9pfs function
  *   2. Set buflen=0 so alloc_ep_req fails -> alloc_requests fails
  *      -> in_req freed, NOT NULLed (dangling pointer)
  *   3. Link function, enable UDC
  *   4. Disable UDC -> unbind -> usb9pfs_clear_tx -> CRASH
  *
  * Build:  gcc -Wall -O2 -o poc poc.c
  * Run:    ./poc   (root, KASAN-enabled kernel, dummy_hcd loaded)
  */

#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <stdarg.h>
#include <fcntl.h>

static int do_cmd(const char *fmt, ...)
{
     char cmd[1024];
     va_list ap;
     va_start(ap, fmt);
     vsnprintf(cmd, sizeof(cmd), fmt, ap);
     va_end(ap);
     return system(cmd);
}

int main(void)
{
     printf("=== USB9PFS Dangling Pointer PoC ===\n\n");

     system("rm -rf /sys/kernel/config/usb_gadget/g1 2>/dev/null");
     system("rmdir /sys/kernel/config/usb_gadget/g1 2>/dev/null");

     /* Create gadget */
     do_cmd("mkdir -p /sys/kernel/config/usb_gadget/g1/configs/c.1/"
            "strings/0x409");
     do_cmd("mkdir -p /sys/kernel/config/usb_gadget/g1/functions/"
            "usb9pfs.gg");
     do_cmd("mkdir -p /sys/kernel/config/usb_gadget/g1/strings/0x409");
     do_cmd("echo 0x1d6b > "
            "/sys/kernel/config/usb_gadget/g1/idVendor");
     do_cmd("echo 0x0104 > "
            "/sys/kernel/config/usb_gadget/g1/idProduct");
     do_cmd("echo 0x0200 > "
            "/sys/kernel/config/usb_gadget/g1/bcdUSB");
     do_cmd("echo 1234 > /sys/kernel/config/usb_gadget/g1/strings/"
            "0x409/serialnumber");
     do_cmd("echo test > /sys/kernel/config/usb_gadget/g1/strings/"
            "0x409/manufacturer");
     do_cmd("echo test > /sys/kernel/config/usb_gadget/g1/strings/"
            "0x409/product");
     do_cmd("echo Config1 > /sys/kernel/config/usb_gadget/g1/configs/"
            "c.1/strings/0x409/configuration");

     /* buflen=0 causes alloc_ep_req() -> alloc_requests() failure */
     printf("[*] Set buflen=0\n");
     do_cmd("echo 0 > /sys/kernel/config/usb_gadget/g1/functions/"
            "usb9pfs.gg/buflen");

     /* Link function */
     printf("[*] Link function\n");
     do_cmd("ln -s /sys/kernel/config/usb_gadget/g1/functions/"
            "usb9pfs.gg "
            "/sys/kernel/config/usb_gadget/g1/configs/c.1/");

     /* Enable: in_req freed but not NULLed */
     printf("[*] Enable UDC\n");
     do_cmd("echo dummy_udc.0 > "
            "/sys/kernel/config/usb_gadget/g1/UDC");
     sleep(1);

     /* Disable: triggers unbind -> usb9pfs_clear_tx -> KASAN */
     printf("[*] Disable UDC (expect KASAN report)\n");
     do_cmd("echo '' > /sys/kernel/config/usb_gadget/g1/UDC");
     sleep(1);

     printf("[*] Done. Check dmesg for KASAN null-ptr-deref.\n");
     return 0;
}
---8<---

Step 2 triggers the fail_in error path in alloc_requests():

   static int alloc_requests(struct f_usb9pfs *usb9pfs)
   {
       usb9pfs->in_req = usb_ep_alloc_request(usb9pfs->in_ep, GFP_KERNEL);
       ...
       usb9pfs->out_req = alloc_ep_req(usb9pfs->out_ep, usb9pfs->buflen);
       if (!usb9pfs->out_req)     // buflen=0 causes this to fail
           goto fail_in;
       ...
   fail_in:
       usb_ep_free_request(usb9pfs->in_ep, usb9pfs->in_req);
       // BUG: usb9pfs->in_req is NOT set to NULL here
   fail:
       return ret;
   }

usb9pfs->in_req now points to freed memory.  In step 4, the composite
framework calls usb9pfs_disable() -> usb9pfs_clear_tx(), which does:

   guard(spinlock_irqsave)(&usb9pfs->lock);
   req = usb9pfs->in_req->context;   // dangling pointer dereference

This is a regression from a3be076dc174 ("net/9p/usbg: Add new usb gadget
function transport").  The fix is to set usb9pfs->in_req = NULL after
freeing it in the error path:

   fail_in:
       usb_ep_free_request(usb9pfs->in_ep, usb9pfs->in_req);
+     usb9pfs->in_req = NULL;
   fail:
       return ret;

A prior review on Sashiko[1] also identified this issue and noted
several other problems in the same file (double-free in the
disable_usb9pfs() path after an alloc_requests failure, missing cleanup
in tx/rx completion error paths, and a potential deadlock in
p9_usbg_request).  The alloc_requests error path NULL fix is the
minimum fix needed for the crash reported here.

[1] 
https://sashiko.dev/#/patchset/20260607130118.16579-1-zhaoyz24%40mails.tsinghua.edu.cn


Hope this is helpful for further fix, thanks.


Best,

Xiao

Re: [PATCH] net/9p/usbg: Fix use-after-free in disable_usb9pfs()

[Yizhou Zhao]

Hi Xiao,

Thanks for the PoC. I re-tested it on a fresh upstream tree
(2d3090a8aeb5, 7.1.0-rc7-00016-g2d3090a8aeb5) with KASAN and lockdep
enabled, and I can reproduce the reported call trace:

  strcmp
  look_up_lock_class
  register_lock_class
  __lock_acquire
  lock_acquire
  __wake_up
  p9_client_cb
  usb9pfs_clear_tx

Without lockdep, the same PoC reaches the same usb9pfs_clear_tx() ->
p9_client_cb() -> __wake_up() path, but it shows up as an RCU stall
instead of the lockdep/strcmp crash.

On Sat, Jun 13, 2026 at 04:27:38AM +0800, XIAO WU wrote:
> I wrote the following PoC to trigger this bug.  It creates a USB
> gadget with a usb9pfs function, sets buflen=0 so that alloc_ep_req()
> fails in alloc_requests(), which frees in_req without NULLing the
> pointer, then unbinds the gadget to trigger usb9pfs_clear_tx() on the
> dangling in_req.

However, I think the actual trigger is slightly different from the
allocation-failure path described in the mail. Setting buflen to 0 does
not make alloc_ep_req() fail in my test: usb_ep_align(..., 0) produces a
zero length, and kmalloc(0) returns ZERO_SIZE_PTR rather than NULL. So
alloc_requests() still succeeds.

The failure seems to happen because in_req->context is still initialized
as the f_usb9pfs pointer. During disconnect, usb9pfs_clear_tx() treats
that context as a struct p9_req_t * and passes it to p9_client_cb(),
which eventually calls wake_up() on a bogus req->wq. With lockdep
enabled, that bogus waitqueue/lock state leads to the strcmp ->
register_lock_class() crash.

So I agree that the PoC exposes a real usb9pfs teardown/context bug, but
it looks independent from the endpoint-disable/free-ordering UAF fixed by
my patch. I think it would be clearer to handle it in a separate patch, 
with a commit message describing the actual trigger path.

Thanks,
Yizhou

Re: [PATCH] net/9p/usbg: Fix use-after-free in disable_usb9pfs()

[Michael Grzeschik]

On Sun, Jun 07, 2026 at 09:01:16PM +0800, Yizhou Zhao wrote:
> disable_usb9pfs() frees the IN and OUT usb_request objects before it
> disables the corresponding endpoints. If either request is still queued,
> the later usb_ep_disable() call cancels the endpoint queue and the UDC
> driver can still access the already freed request.
> 
> With dummy_hcd and KASAN, this is reproducible by queueing the OUT
> request and then disconnecting the configfs gadget:
> 
>   BUG: KASAN: slab-use-after-free in dummy_disable+0x2b4/0x300
>   Read of size 8 at addr ffff888009702400 by task sh/1
>   usb_ep_disable+0x8e/0x1f0
>   usb9pfs_func_unbind+0x193/0x350
>   gadget_dev_desc_UDC_store+0x135/0x280
> 
> dummy_free_request() also warns because the request is freed while its
> queue entry is still linked.
> 
> Disable both endpoints before freeing the request objects. This lets
> usb_ep_disable() cancel any queued transfers and invoke the completion
> callback while the request storage is still valid. The request objects
> are then freed only after they have been removed from the endpoint
> queues.

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

> 
> Fixes: a3be076dc174 ("net/9p/usbg: Add new usb gadget function transport")
> Cc: stable@vger.kernel.org
> 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>
> ---
>  net/9p/trans_usbg.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/net/9p/trans_usbg.c b/net/9p/trans_usbg.c
> index 1ce70338999c..5d0d6add150e 100644
> --- a/net/9p/trans_usbg.c
> +++ b/net/9p/trans_usbg.c
> @@ -278,6 +278,9 @@ static void disable_usb9pfs(struct f_usb9pfs *usb9pfs)
>  	struct usb_composite_dev *cdev =
>  		usb9pfs->function.config->cdev;
>  
> +	disable_ep(cdev, usb9pfs->in_ep);
> +	disable_ep(cdev, usb9pfs->out_ep);
> +
>  	if (usb9pfs->in_req) {
>  		usb_ep_free_request(usb9pfs->in_ep, usb9pfs->in_req);
>  		usb9pfs->in_req = NULL;
> @@ -287,9 +290,6 @@ static void disable_usb9pfs(struct f_usb9pfs *usb9pfs)
>  		usb_ep_free_request(usb9pfs->out_ep, usb9pfs->out_req);
>  		usb9pfs->out_req = NULL;
>  	}
> -
> -	disable_ep(cdev, usb9pfs->in_ep);
> -	disable_ep(cdev, usb9pfs->out_ep);
>  	dev_dbg(&cdev->gadget->dev, "%s disabled\n",
>  		usb9pfs->function.name);
>  }
> -- 
> 2.43.0
>