Re: [PATCH v4 0/9] PCI: Fix UAF and TOCTOU related to dynamic ID
From: Masanori Goto <gotom@debian.or.jp>
Date: 2026-08-01 03:13:48
Also in:
dri-devel, driver-core, linux-ide, linux-pci, linux-scsi, lkml
I'm a "nsp32 folk", which patch version should I take a look at? Best regards, Masanori 2026年7月29日(水) 1:27 Bjorn Helgaas [off-list ref]:
On Thu, Jul 23, 2026 at 11:00:39PM +0100, Gary Guo wrote:quoted
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/20260618-id_info-v1-0-96af1e559ef9@garyguo.net/ (local) [1] Link: https://lore.kernel.org/all/20260619170503.518F61F00A3A@smtp.kernel.org/ (local) [2] --- Changes in v4: - Code and commit message style fixes (Bjorn) - Link to v3: https://patch.msgid.link/20260706-pci_id_fix-v3-0-2d48fc025acc@garyguo.net 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/20260630-pci_id_fix-v2-0-b834a98c0af2@garyguo.net 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/20260626-pci_id_fix-v1-0-a35c803f1b95@garyguo.net --- 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.