Thread (9 messages) 9 messages, 2 authors, 1d ago
WARM1d
Revisions (2)
  1. v1 [diff vs current]
  2. v2 current

[PATCH v2 0/7] usb: fix UAF related to dynamic ID

From: Gary Guo <gary@garyguo.net>
Date: 2026-07-07 12:27:01
Also in: driver-core, linux-media, linux-usb, linux-wireless, lkml

This is the USB version of the dynamic ID UAF fix similar to that of PCI
[1]. usb_match_dynamic_id returns a pointer to field of usb_dynid, which
can be freed when dynamic ID is removed via sysfs. It can be triggered with
the following sequence:

    echo <vid> <pid> > /sys/bus/usb/drivers/<name>/new_id
    <probe start>
    echo <vid> <pid> > /sys/bus/usb/drivers/<name>/remove_id
    <probe use ID>

Fix it by making a stack copy of the ID. This does mean that the lifetime
of ID is scoped to probe (which is already the case but never spelled out
explicitly). Drivers use these device IDs creatively, so this series also
fix these drivers.

The following coccinelle script is used to find all cases that are deemed
suspicious. Only useful case for IDs should be access its fields, or
forwarding (without type cast) to functions that do so.

@usage@
identifier fn, id;
position p;
@@
  fn(..., struct usb_device_id *id, ...)
  {
    ...
    id@p
    ...
  }

// Due to cocci isomorphism this needs to be explicit
@bad@
identifier fn, id;
type T;
position usage.p;
@@
  fn(..., struct usb_device_id *id, ...)
  {
    ...
    (T*)id@p
    ...
  }

// Good use cases
@good@
identifier fn, id, fld;
expression E;
position usage.p;
@@
  fn(..., struct usb_device_id *id, ...)
  {
    ...
(
    id@p->fld
|
    E(..., id@p, ...)
|
// Redundant checks, but ignore
    !id@p
|
// Redundant checks, but ignore
    id ? ... : ...
)
    ...
  }

@script:python depends on usage && (bad || !good)@
p << usage.p;
@@
coccilib.report.print_report(p[0], "suspicious use of device ID")

There're 3 drivers that store usb_device_id, and they're converted to just
use driver_info instead. The other fields of usb_device_id can be easily
retrieved from usb_device via descriptor.id{Vendor,Product}.

There're also a few users that rely on pointer arithmetics. Pegaus and
xusbatm are converted to use driver_info. All unusal USB mass storage
drivers rely on pointer arithemtic to index into a side table, because USB
storage subsystem is using the driver_data for flags. Luckily all these
drivers set no_dynamic_id. Ideally these could be fixed too but their
maintainers probably have a better idea of how.

Link: https://lore.kernel.org/driver-core/20260706-pci_id_fix-v3-0-2d48fc025acc@garyguo.net (local) [1]

Signed-off-by: Gary Guo <gary@garyguo.net>
---
Changes in v2:
- Add le16_to_cpu in ath9k patch.
- Fix AS102 which relies on address comparison. (Sashiko)
- Fix pegasus, xusbatm which relies on pointer arithmetic.
- usb_device_id from static ID table is now not copied. This ensures that
  USB mass storage subsystem can still perform pointer arithmetic. This
  also addresses Danilo's review feedback.
- Link to v1: https://patch.msgid.link/20260630-usb_dyn_id_uaf-v1-0-160a02be5ac2@garyguo.net

---
Gary Guo (7):
      wifi: ath9k_htc: don't store usb_device_id
      usb: usbtmc: don't store usb_device_id
      usb: serial: spcp8x5: don't store usb_device_id
      media: as102: do not rely on id table address comparison
      net: usb: pegasus: don't rely on id table pointer arithmetic
      usb: xusbatm: don't rely on id table pointer arithmetic
      usb: fix UAF when probe runs concurrent to dyn ID removal

 drivers/media/usb/as102/as102_usb_drv.c  | 73 +++++++++++++-------------------
 drivers/net/usb/pegasus.c                | 54 ++++++++++-------------
 drivers/net/usb/pegasus.h                |  3 --
 drivers/net/wireless/ath/ath9k/hif_usb.c | 12 +++---
 drivers/net/wireless/ath/ath9k/hif_usb.h |  2 +-
 drivers/usb/atm/xusbatm.c                |  6 ++-
 drivers/usb/class/usbtmc.c               |  2 -
 drivers/usb/core/driver.c                | 12 ++++--
 drivers/usb/serial/spcp8x5.c             |  6 +--
 include/linux/usb.h                      |  3 +-
 10 files changed, 76 insertions(+), 97 deletions(-)
---
base-commit: 2b763db0c2763d6bf73d7d3e69665222d1f377cf
change-id: 20260629-usb_dyn_id_uaf-9d5f415387d4

Best regards,
--  
Gary Guo [off-list ref]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help