Netdev List
 help / color / mirror / Atom feed
* [PATCH v3 0/9] pci: fix UAF and TOCTOU related to dynamic ID
@ 2026-07-06 14:11 Gary Guo
  2026-07-06 14:11 ` [PATCH v3 1/9] ata: don't store pci_device_id Gary Guo
                   ` (8 more replies)
  0 siblings, 9 replies; 11+ messages in thread
From: Gary Guo @ 2026-07-06 14:11 UTC (permalink / raw)
  To: Bjorn Helgaas, Zhenzhong Duan, Greg Kroah-Hartman,
	Rafael J. Wysocki, Danilo Krummrich, Damien Le Moal,
	Niklas Cassel, GOTO Masanori, YOKOTA Hiroshi,
	James E.J. Bottomley, Martin K. Petersen, Vaibhav Gupta,
	Jens Taprogge, Ido Schimmel, Petr Machata, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	David Airlie
  Cc: linux-pci, driver-core, linux-kernel, linux-ide, linux-scsi,
	industrypack-devel, netdev, dri-devel, Gary Guo, Sashiko

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/ [1]
Link: https://lore.kernel.org/all/20260619170503.518F61F00A3A@smtp.kernel.org/ [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/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: 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 <gary@garyguo.net>


^ permalink raw reply	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-07-07  1:12 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-06 14:11 [PATCH v3 0/9] pci: fix UAF and TOCTOU related to dynamic ID Gary Guo
2026-07-06 14:11 ` [PATCH v3 1/9] ata: don't store pci_device_id Gary Guo
2026-07-07  1:12   ` Damien Le Moal
2026-07-06 14:11 ` [PATCH v3 2/9] nsp32: " Gary Guo
2026-07-06 14:11 ` [PATCH v3 3/9] ipack: tpci200: " Gary Guo
2026-07-06 14:11 ` [PATCH v3 4/9] mlxsw: " Gary Guo
2026-07-06 14:11 ` [PATCH v3 5/9] agp/via: don't rely on address of pci_device_id Gary Guo
2026-07-06 14:11 ` [PATCH v3 6/9] agp/amd-k7: " Gary Guo
2026-07-06 14:11 ` [PATCH v3 7/9] pci: make pci_match_one_device match on ID instead of device Gary Guo
2026-07-06 14:11 ` [PATCH v3 8/9] pci: fix dyn_id add TOCTOU Gary Guo
2026-07-06 14:11 ` [PATCH v3 9/9] pci: fix UAF when probe runs concurrent to dyn ID removal Gary Guo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox