Re: [PATCH 3/3] pci: fix UAF when probe runs concurrent to dyn ID removal

[Gary Guo]

On Fri Jun 26, 2026 at 8:49 PM BST, Gary Guo wrote:
> Dynamic IDs are only guaranteed to be valid when dynids.lock is held,
> as remove_id_store can free the node. Thus, make a copy in
> pci_match_device.
>
> Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
> Link: https://lore.kernel.org/all/20260619170503.518F61F00A3A@xxxxxxxxxxxxxxx/
> Fixes: 0994375e9614 ("PCI: add remove_id sysfs entry")
> Signed-off-by: Gary Guo <gary@xxxxxxxxxxx>

Sashiko reports driver UAF issue with this:
https://sashiko.dev/#/patchset/20260626-pci_id_fix-v1-0-a35c803f1b95%40garyguo.net?part=3

All of these cases are already existing UAF issues, albeit now it is much easier
to trigger because this patch converts them to heap UAF for dynamic ID only to
stack UAF for all IDs.

One obvious fix is to instead keep a copy of the ID in pci_dev. However, in
practice I think it doesn't make much sense for drivers to keep pci_device_id
pointers at all; everything except driver_data can just be retrieved from
pci_dev.

So I think it's better to fix drivers that stores these IDs. I used this
coccinelle script to find cases:

---
@store@
identifier fn;
identifier id;
expression E;
parameter list[n] ps;
@@
  fn(ps, const struct pci_device_id *id, ...)
  {
    ...
*   E = id
    ...
  }

@store_cast@
identifier fn;
identifier id;
expression E;
type T;
parameter list[n] ps;
@@
  fn(ps, const struct pci_device_id *id, ...)
  {
    ...
*   E = (T)id
    ...
  }

@struct_const@
identifier s, fld;
@@
  struct s {
    ...
*   const struct pci_device_id *fld;
    ...
  };

@struct_mut@
identifier s, fld;
@@
  struct s {
    ...
*   struct pci_device_id *fld;
    ...
  };
---

And it looks like there're only 4 drivers that need fixing:
- mlxsw
- ipack/tpci200
- nsp32
- ata

Best,
Gary