Re: [PATCH] usb: gadget: udc: Fix NULL pointer dereference in gadget_match_driver

[Jimmy Hu (xWF)]

On Tue, Jun 2, 2026 at 10:30 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, Jun 02, 2026 at 01:34:07PM +0800, Jimmy Hu (xWF) wrote:
> > On Wed, May 27, 2026 at 2:00 AM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > On Tue, May 26, 2026 at 03:06:35PM +0800, Jimmy Hu wrote:
> > > > A NULL pointer dereference occurs in gadget_match_driver() because a
> > > > race condition exists between the DRD mode-switch work and the
> > > > configfs UDC write path:
> > > >
> > > > 1. The DRD mode-switch work invokes __dwc3_set_mode(), which calls
> > > >    dwc3_gadget_exit() and subsequently frees the UDC device name via
> > > >    device_unregister(&udc->dev).
> > > > 2. The configfs UDC write path invokes gadget_dev_desc_UDC_store(),
> > > >    which calls usb_gadget_register_driver() and subsequently
> > > >    compares the UDC device name via gadget_match_driver().
> > > >
> > > > If gadget_match_driver() runs concurrently during UDC unregistration, it
> > > > may access the freed UDC device name. Once the freed memory is zeroed,
> > > > dev_name(&udc->dev) returns NULL, causing a panic in strcmp().
> > >
> > > I don't see how this can happen.  gadget_match_driver() runs during
> > > probing of a gadget, which takes place only while the gadget is
> > > registered in the device core.  But usb_del_gadget() calls
> > > device_del(&gadget->dev) before it calls device_unregister(&udc->dev).
> > > This means that at any time when gadget_match_driver() can run, the UDC
> > > device name must still be allocated.
> > >
> > > You should run more tests.  Add debugging printk() calls just before and
> > > just after the device_del(&gadget->dev) and device_unregister(&udc->dev)
> > > lines, and inside gadget_match_driver(), so the tests will show
> > > unambiguously when these things happen with respect to each other.
> > >
> > > > Fix this by checking dev_name(&udc->dev) before calling strcmp().
> > >
> > > Adding a check like this will not fix a race; it will only make the race
> > > less likely to occur.  It won't prevent the name from being deallocated
> > > between the check and the strcmp() call.
> > >
> > > Alan Stern
> >
> > Hi Alan,
> >
> > Thank you for the review. You are absolutely right about the TOCTOU risk;
> > the simple NULL check does not prevent the name from being deallocated
> > after the check but before the strcmp() call.
> >
> > I will submit a v2 patch that uses get_device(&udc->dev) and put_device()
> > to increment the UDC reference count during the matching phase. This will
> > guarantee that the UDC device name remains allocated and valid throughout
> > the entire duration of strcmp(), eliminating the race condition structurally.
> >
> > Does this approach sound reasonable to you?
>
> No, because you haven't addressed the issue I raised at the start of my
> email, namely, how can this problem actually occur?  And you didn't run
> additional tests with the extra debugging information that I asked for.
>
> Alan Stern

Hi Alan,

I have captured the KASAN log with the extra debugging information
you requested, which shows how this race condition occurs.

The log shows that after gadget_match_driver() enters execution on
one core, a parallel core can invoke usb_del_gadget() and complete
both device_del(&gadget->dev) and device_unregister(&udc->dev)
before strcmp() executes.

Here is the exact timeline from the dmesg output:

1. At 268.595241, task 1374 (configfs path) enters
gadget_match_driver() (on CPU6):
[  268.595241][ T1374] [CPU6
android.hardwar]:[JJ][core.c/gadget_match_driver/1568] Enter

2. At 268.595250 (only 9 us later), DRD work invokes usb_del_gadget() (on CPU3):
[  268.595250][  T102] [CPU3
kworker/3:1]:[JJ][core.c/usb_del_gadget/1529] Before
device_del(&gadget->dev);
[  268.598129][  T102] [CPU3
kworker/3:1]:[JJ][core.c/usb_del_gadget/1531] After
device_del(&gadget->dev);
[  268.598159][  T102] [CPU3
kworker/3:1]:[JJ][core.c/usb_del_gadget/1534] Before
device_unregister(&udc->dev);
[  268.599405][  T102] [CPU3
kworker/3:1]:[JJ][core.c/usb_del_gadget/1536] After
device_unregister(&udc->dev);

3. At 268.599427, task 1374 starts comparison, where it triggers a
KASAN invalid-access. (Due to kernel preemption, the task was migrated
to CPU7):
[  268.599434][ T1374] BUG: KASAN: invalid-access in
gadget_match_driver+0x150/0x1cc
[  268.599448][ T1374] Read of size 8 at addr 66ffff801a49b880 by task
android.hardwar/1374
[  268.599454][ T1374] Pointer tag: [66], memory tag: [fe]
[  268.599456][ T1374]
[  268.599460][ T1374] CPU: 7 PID: 1374 Comm: android.hardwar Tainted:
G S      W  O       6.1.124-android14-11-ga633402dff84-dirty #1

To resolve this object lifetime issue, I see two potential approaches:

1. Protect the UDC device lifecycle during the comparison phase using
   get_device(&udc->dev) and put_device() (as a lightweight fix).
2. Serialize the configfs match path and the unregister path using
   a subsystem mutex.

I would highly appreciate your thoughts on which direction you prefer.
Does this data address the scenario you raised?

Thanks,
Jimmy