usb: gadget: composite: dead empty check in USB_DT_OTG handler?

[Maoyi Xie]

While auditing drivers/usb/gadget/composite.c, I noticed what
looks like a dead empty check in the OTG branch of
composite_setup(). I built an end to end reproducer that crashes
the kernel under KASAN. I would appreciate it if you could take a
look and let me know whether this is worth fixing.

The site is composite_setup() handling USB_DT_OTG (linux-7.1-rc1,
around line 1858):

    case USB_DT_OTG:
            if (gadget_is_otg(gadget)) {
                    struct usb_configuration *config;
                    int otg_desc_len = 0;

                    if (cdev->config)
                            config = cdev->config;
                    else
                            config = list_first_entry(
                                            &cdev->configs,
                                    struct usb_configuration, list);
                    if (!config)
                            goto done;

                    ...

                    value = min_t(int, w_length, otg_desc_len);
                    memcpy(req->buf, config->descriptors[0], value);
            }
            break;

The `if (!config) goto done;` looks like a guard for an empty
cdev->configs. list_first_entry() never returns NULL though. It
returns container_of(&cdev->configs, struct usb_configuration,
list), which aliases the list head. The check is dead code.

With an empty cdev->configs, config aliases &cdev->configs inside
struct usb_composite_dev. The read of config->descriptors[0]
fetches memory at a fixed offset from cdev. memcpy() then copies
up to w_length bytes from that location into req->buf.

Empty cdev->configs is reachable in two cases. One is a gadget
driver that returns from composite_bind without calling
usb_add_config(). The other is a teardown race during gadget
unbind while a control transfer is in flight.

End to end reproducer on linux-7.0 with KASAN, CONFIG_USB_GADGET
and CONFIG_USB_DUMMY_HCD:

  - g_empty.ko: minimal composite gadget. Sets gadget->is_otg in
    bind(). Does not call usb_add_config(). cdev->configs stays
    empty for the lifetime of the gadget.
  - kprobe on composite_setup(): rewrites the first incoming
    GET_DESCRIPTOR to USB_DT_OTG so the function enters the
    vulnerable branch deterministically.

KASAN report from the unpatched kernel:

  [probe] composite_setup: bReq=GET_DESCRIPTOR wValue=0x0900 desc_type=9
  [probe]   cdev=ffff8881106f5400 cdev->config=NULL configs_empty=1
  [probe] BUG WINDOW: list_first_entry on empty configs returns
          ffff8881106f5520 (head=ffff8881106f5558 diff=-56 bytes)
  Oops: general protection fault, probably for non canonical address
        0xdffffc0000000000: 0000 [#1] SMP KASAN PTI
  KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
  RIP: 0010:composite_setup+0x38a1/0x77d0
  Kernel panic - not syncing: Fatal exception in interrupt

A candidate fix is a one liner. Replace list_first_entry with
list_first_entry_or_null so the existing NULL guard runs:

    -                       config = list_first_entry(
    -                                       &cdev->configs,
    -                               struct usb_configuration, list);
    +                       config = list_first_entry_or_null(
    +                                       &cdev->configs,
    +                                       struct usb_configuration,
    +                                       list);

I verified the patched kernel on the same reproducer. KASAN does
not trip. composite_setup takes the `goto done` branch cleanly.

Similar dead empty checks after list_first_entry have been
cleaned up in the same shape, for example commit fbb8bc408027
(net: qed: Remove redundant NULL checks after list_first_entry),
commit c708d3fad421 (crypto: atmel: use list_first_entry_or_null
to simplify find_dev) and commit 10379171f346 (ksmbd: use
list_first_entry_or_null for opinfo_get_list). The qed commit
message describes the exact shape we observe here. This OTG site
appears to be missed by those cleanups.

If this is intentional or already known, please disregard.
Otherwise I am happy to send a [PATCH] or to leave the fix to
you.

Thanks,
Maoyi Xie
https://maoyixie.com/