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

[Jimmy Hu (xWF)]

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?

Thanks,
Jimmy