Re: [PATCH v4 0/9] PCI: Fix UAF and TOCTOU related to dynamic ID
Masanori Goto <[email protected]> Sat, 1 Aug 2026 12:13:18 +0900
| Newsgroups | org.kernel.vger.linux-pci,dev.linux.lists.driver-core,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-ide,org.kernel.vger.linux-kernel,org.kernel.vger.linux-scsi,org.kernel.vger.netdev |
|---|---|
| Message-ID | <CALZLnaEmm5Zi=30n4i7ioX9ii4KMAePnZTAhAcNScSZFSNB7UA@mail.gmail.com> |
I'm a "nsp32 folk", which patch version should I take a look at? Best regards, Masanori 2026年7月29日(水) 1:27 Bjorn Helgaas <[email protected]>: > > On Thu, Jul 23, 2026 at 11:00:39PM +0100, 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. > > > > @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 ignore > > 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 v4: > > - Code and commit message style fixes (Bjorn) > > - Link to v3: https://patch.msgid.link/[email protected] > > > > 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: ata_generic: don't store pci_device_id > > scsi: nsp32: don't store pci_device_id > > ipack: tpci200: don't store pci_device_id > > mlxsw: pci: 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 | 194 ++++++++++--------- > > drivers/pci/pci.h | 43 +++-- > > drivers/pci/search.c | 8 +- > > drivers/scsi/nsp32.c | 8 +- > > drivers/scsi/nsp32.h | 8 +- > > include/linux/pci.h | 1 + > > 12 files changed, 237 insertions(+), 378 deletions(-) > > I put this on pci/enumeration and plan it for v7.3, hopefully with > acks from the AGP, tpci200, and nsp32 folks.