Re: [PATCH v3 0/9] pci: fix UAF and TOCTOU related to dynamic ID
"Gary Guo" <[email protected]> Tue, 21 Jul 2026 14:17:23 +0100
| Newsgroups | gmane.linux.scsi,gmane.linux.kernel.pci,gmane.linux.kernel,gmane.linux.ide,gmane.linux.kernel.ipack,gmane.linux.network,gmane.comp.video.dri.devel |
|---|---|
| Message-ID | <[email protected]> |
On Mon Jul 6, 2026 at 3:11 PM BST, Gary Guo wrote: > While working on improving the Rust abstractions [1], Sashiko reported that > an existing UAF issue related to dynamic ID, which I find to be genuine. > When taking a look at the code I also find a TOCTOU issue where the > existence check of dynamic ID happens in a separate critical section as the > actual insertion. This series fix both issues. > > There are two exported functions "pci_match_id" and "pci_add_dynid" which I > have to tweak to implement this cleanly; I created separate "do_xxx" > functions to keep the existing APIs because they all have multiple users. > > There're a few existing users which stores their pci_device_id argument in > probe callback. This is a bad pattern because nothing except driver_data > inside pci_device_id is what they want; actual ID information can be > retrieved from pci_dev instead. > > There are two users that performs pointer arithmetic on the pci_device_id; > these are also problematic with dynamic ID and driver_override, so fix them > as well. > > I've used the following coccinelle script to flag all cases where the > pci_device_id is used other than reading its fields. Hi Bjorn, Could you take a look at the series? For reference, this series is the USB equivalent which is already applied: https://lore.kernel.org/driver-core/[email protected]/ Thanks, Gary > > @usage@ > identifier fn, id; > position p; > @@ > fn(..., struct pci_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 pci_device_id *id, ...) > { > ... > (T*)id@p > ... > } > > // Good use cases > @good@ > identifier fn, id, fld; > expression E; > position usage.p; > @@ > fn(..., struct pci_device_id *id, ...) > { > ... > ( > id@p->fld > | > E(..., id@p, ...) > | > // Redundant checks, but ignore > !id@p > | > // Redundant checks, but ignorehttps://lore.kernel.org/driver-core/[email protected]/ > id ? ... : ... > ) > ... > } > > @script:python depends on usage && (bad || !good)@ > p << usage.p; > @@ > coccilib.report.print_report(p[0], "suspicious use of pci_device_id") > > Link: https://lore.kernel.org/all/[email protected]/ [1] > Link: https://lore.kernel.org/all/[email protected]/ [2] > > --- > Changes in v3: > - Fix users which uses pci_device_id for pointer arithmetic. (Sashiko) > - Convert to scoped_guard. (Danilo) > - For static IDs, still give out static pointers and avoid making a copy. > - Link to v2: https://patch.msgid.link/[email protected] > > Changes in v2: > - Fix users which store pci_device_id. > - Clarify in probe documentation about the lifetime of pci_device_id > parameter. > - Dynamic ID conflict check now ignores override_only. (Sashiko) > - Link to v1: https://patch.msgid.link/[email protected] > > --- > Gary Guo (9): > ata: don't store pci_device_id > nsp32: don't store pci_device_id > ipack: tpci200: don't store pci_device_id > mlxsw: don't store pci_device_id > agp/via: don't rely on address of pci_device_id > agp/amd-k7: don't rely on address of pci_device_id > pci: make pci_match_one_device match on ID instead of device > pci: fix dyn_id add TOCTOU > pci: fix UAF when probe runs concurrent to dyn ID removal > > drivers/ata/ata_generic.c | 6 +- > drivers/char/agp/amd-k7-agp.c | 26 +-- > drivers/char/agp/via-agp.c | 308 +++++++----------------------- > drivers/ipack/carriers/tpci200.c | 1 - > drivers/ipack/carriers/tpci200.h | 1 - > drivers/net/ethernet/mellanox/mlxsw/pci.c | 11 +- > drivers/pci/pci-driver.c | 193 ++++++++++--------- > drivers/pci/pci.h | 36 +++- > drivers/pci/search.c | 6 +- > drivers/scsi/nsp32.c | 8 +- > drivers/scsi/nsp32.h | 8 +- > include/linux/pci.h | 1 + > 12 files changed, 230 insertions(+), 375 deletions(-) > --- > base-commit: 2b763db0c2763d6bf73d7d3e69665222d1f377cf > change-id: 20260626-pci_id_fix-83eaec007674 > > Best regards, > -- > Gary Guo <[email protected]>