[PATCH v2 0/7] pci: fix UAF and TOCTOU related to dynamic ID

[PATCH v2 0/7] pci: fix UAF and TOCTOU related to dynamic ID

[Gary Guo]

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. I've used the following coccinelle script
to find the cases where the argument is stored and converted them to stop
storing pci_device_id.

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

@cast@
identifier fn;
identifier id;
parameter list[n] ps;
@@
  fn(ps, struct pci_device_id *id, ...)
  {
    ...
*   (void *)id
    ...
  }

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

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 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 (7):
      ata: don't keep pci_device_id
      nsp32: don't keep pci_device_id
      ipack: tpci200: don't keep pci_device_id
      mlxsw: don't keep 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/ipack/carriers/tpci200.c          |   1 -
 drivers/ipack/carriers/tpci200.h          |   1 -
 drivers/net/ethernet/mellanox/mlxsw/pci.c |  11 +-
 drivers/pci/pci-driver.c                  | 219 ++++++++++++++++--------------
 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 +
 10 files changed, 166 insertions(+), 131 deletions(-)
---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260626-pci_id_fix-83eaec007674

Best regards,
--  
Gary Guo <gary@garyguo.net>

[PATCH v2 1/7] ata: don't keep pci_device_id

[Gary Guo]

pci_device_id is not guaranteed to live longer than probe due to presence
of dynamic ID. All information apart from driver_data can be easily
retrieved from pci_dev, so just store driver_data.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 drivers/ata/ata_generic.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/ata/ata_generic.c b/drivers/ata/ata_generic.c
index e70b6c089cf1..18ea740ca582 100644
--- a/drivers/ata/ata_generic.c
+++ b/drivers/ata/ata_generic.c
@@ -51,11 +51,11 @@ enum {
 static int generic_set_mode(struct ata_link *link, struct ata_device **unused)
 {
 	struct ata_port *ap = link->ap;
-	const struct pci_device_id *id = ap->host->private_data;
+	unsigned long driver_data = (unsigned long)ap->host->private_data;
 	int dma_enabled = 0;
 	struct ata_device *dev;
 
-	if (id->driver_data & ATA_GEN_FORCE_DMA) {
+	if (driver_data & ATA_GEN_FORCE_DMA) {
 		dma_enabled = 0xff;
 	} else if (ap->ioaddr.bmdma_addr) {
 		/* Bits 5 and 6 indicate if DMA is active on master/slave */
@@ -206,7 +206,7 @@ static int ata_generic_init_one(struct pci_dev *dev, const struct pci_device_id
 			return rc;
 		pcim_pin_device(dev);
 	}
-	return ata_pci_bmdma_init_one(dev, ppi, &generic_sht, (void *)id, 0);
+	return ata_pci_bmdma_init_one(dev, ppi, &generic_sht, (void *)id->driver_data, 0);
 }
 
 static const struct pci_device_id ata_generic[] = {

-- 
2.54.0

Re: [PATCH v2 1/7] ata: don't keep pci_device_id

[Niklas Cassel]

Hello Gary,

On Tue, Jun 30, 2026 at 12:09:01PM +0100, Gary Guo wrote:
> pci_device_id is not guaranteed to live longer than probe due to presence
> of dynamic ID. All information apart from driver_data can be easily
> retrieved from pci_dev, so just store driver_data.
> 
> Signed-off-by: Gary Guo <gary@garyguo.net>

Please write a proper commit message.

The commit message should be detailed enough for someone to realize what
is going on without reading your cover-letter (as information in the cover
letter in not part of the accepted commit).

1) Explain how to reproduce.

2) Explain the problem.

3) Explain the consequences of the problem. UAF? Crash?

4) Explain how you fix it.


AFAICT, this is somehow related to pci_add_dynid(), which is called when
user-space is doing something like:

$ echo "vendor device" > /sys/bus/pci/drivers/your_driver/new_id


Kind regards,
Niklas

Re: [PATCH v2 1/7] ata: don't keep pci_device_id

[Gary Guo]

On Tue Jun 30, 2026 at 12:59 PM BST, Niklas Cassel wrote:
> Hello Gary,
>
> On Tue, Jun 30, 2026 at 12:09:01PM +0100, Gary Guo wrote:
>> pci_device_id is not guaranteed to live longer than probe due to presence
>> of dynamic ID. All information apart from driver_data can be easily
>> retrieved from pci_dev, so just store driver_data.
>> 
>> Signed-off-by: Gary Guo <gary@garyguo.net>
>
> Please write a proper commit message.
>
> The commit message should be detailed enough for someone to realize what
> is going on without reading your cover-letter (as information in the cover
> letter in not part of the accepted commit).
>
> 1) Explain how to reproduce.
>
> 2) Explain the problem.
>
> 3) Explain the consequences of the problem. UAF? Crash?
>
> 4) Explain how you fix it.

Hi Niklas,

I see this as a contract mismatch between pci core and drivers, hence the commit
message just mentions the problem (lifetime of pci_device_id pointer is
restricted to probe only) and the fix (don't store it).

Currently as you said, the way that this becomes a problem is when dynamic ID is
involved. So the following sequence will cause issue:

    echo "vendor device" > /sys/bus/pci/drivers/your_driver/new_id
    # PCI core calls probe which stores the ID (e.g. ata)
    echo "vendor device" > /sys/bus/pci/drivers/your_driver/remove_id
    # Driver uses the stored ID (UAF)

However, the gist here is that due to the presence of dynamic ID, pci_device_id
in probe is not guaranteed to live longer than the probe function (in fact, it
currently is not guaranteed to be alive at all, which is what this series is
trying to address).

Exactly how long the ID is going to live should be up to the PCI core and be
transparent to drivers, so I intentionally left this out from driver fix
patches, this should be implementation detail of PCI core. In fact, in patch 7
I changed to be unconditionally invalid upon return regardless if it is dynamic
ID or not.

At the end of this series I changed the documentation to explicitly state this
contract. So even without having the reproducer, the commit message still makes
sense because it fixes a contract violation and reader can connect it with the
documentation.

Best,
Gary

>
>
> AFAICT, this is somehow related to pci_add_dynid(), which is called when
> user-space is doing something like:
>
> $ echo "vendor device" > /sys/bus/pci/drivers/your_driver/new_id
>
>
> Kind regards,
> Niklas

Re: [PATCH v2 1/7] ata: don't keep pci_device_id

[Danilo Krummrich]

On Tue Jun 30, 2026 at 1:09 PM CEST, Gary Guo wrote:
> pci_device_id is not guaranteed to live longer than probe due to presence
> of dynamic ID. All information apart from driver_data can be easily
> retrieved from pci_dev, so just store driver_data.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>

Reviewed-by: Danilo Krummrich <dakr@kernel.org>

Re: [PATCH v2 1/7] ata: don't keep pci_device_id

[sashiko-bot]

> pci_device_id is not guaranteed to live longer than probe due to presence
> of dynamic ID. All information apart from driver_data can be easily
> retrieved from pci_dev, so just store driver_data.
> 
> Signed-off-by: Gary Guo <gary@garyguo.net>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-pci_id_fix-v2-0-b834a98c0af2@garyguo.net?part=1

[PATCH v2 2/7] nsp32: don't keep pci_device_id

[Gary Guo]

pci_device_id is not guaranteed to live longer than probe due to presence
of dynamic ID. All information apart from driver_data can be easily
retrieved from pci_dev, so just store driver_data.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 drivers/scsi/nsp32.c | 8 ++++----
 drivers/scsi/nsp32.h | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/nsp32.c b/drivers/scsi/nsp32.c
index e893d5677241..9c9281222a0a 100644
--- a/drivers/scsi/nsp32.c
+++ b/drivers/scsi/nsp32.c
@@ -1470,7 +1470,7 @@ static int nsp32_show_info(struct seq_file *m, struct Scsi_Host *host)
 		   (nsp32_read2(base, INDEX_REG) >> 8) & 0xff);
 
 	mode_reg = nsp32_index_read1(base, CHIP_MODE);
-	model    = data->pci_devid->driver_data;
+	model    = data->model;
 
 #ifdef CONFIG_PM
 	seq_printf(m, "Power Management:      %s\n",
@@ -2907,8 +2907,8 @@ static int nsp32_eh_host_reset(struct scsi_cmnd *SCpnt)
  */
 static int nsp32_getprom_param(nsp32_hw_data *data)
 {
-	int vendor = data->pci_devid->vendor;
-	int device = data->pci_devid->device;
+	int vendor = data->Pci->vendor;
+	int device = data->Pci->device;
 	int ret, i;
 	int __maybe_unused val;
 
@@ -3340,7 +3340,7 @@ static int nsp32_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	}
 
 	data->Pci	  = pdev;
-	data->pci_devid   = id;
+	data->model       = id->driver_data;
 	data->IrqNumber   = pdev->irq;
 	data->BaseAddress = pci_resource_start(pdev, 0);
 	data->NumAddress  = pci_resource_len  (pdev, 0);
diff --git a/drivers/scsi/nsp32.h b/drivers/scsi/nsp32.h
index 924889f8bd37..9e65771cb592 100644
--- a/drivers/scsi/nsp32.h
+++ b/drivers/scsi/nsp32.h
@@ -564,10 +564,10 @@ typedef struct _nsp32_hw_data {
 
 	struct scsi_cmnd *CurrentSC;
 
-	struct pci_dev             *Pci;
-	const struct pci_device_id *pci_devid;
-	struct Scsi_Host           *Host;
-	spinlock_t                  Lock;
+	struct pci_dev    *Pci;
+	int                model;
+	struct Scsi_Host  *Host;
+	spinlock_t         Lock;
 
 	char info_str[100];
 

-- 
2.54.0

Re: [PATCH v2 2/7] nsp32: don't keep pci_device_id

[Danilo Krummrich]

On Tue Jun 30, 2026 at 1:09 PM CEST, Gary Guo wrote:
> pci_device_id is not guaranteed to live longer than probe due to presence
> of dynamic ID. All information apart from driver_data can be easily
> retrieved from pci_dev, so just store driver_data.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>

Reviewed-by: Danilo Krummrich <dakr@kernel.org>

Re: [PATCH v2 2/7] nsp32: don't keep pci_device_id

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Probe failure incorrectly treated as success leading to NULL pointer dereferences.
- [High] Unvalidated `driver_data` from dynamic PCI IDs causes out-of-bounds reads on the `nsp32_model` array.
- [High] Race condition in `nsp32_probe` due to use of a shared global variable for instance data.
- [High] Double endianness swap in MMIO read/write functions.
--

commit 68e40f6c7308e7ebef3fc97b6095ad0f3e42988d
Author: Gary Guo <gary@garyguo.net>

nsp32: don't keep pci_device_id

pci_device_id is not guaranteed to live longer than probe due to presence
of dynamic ID. All information apart from driver_data can be easily
retrieved from pci_dev, so just store driver_data.

> diff --git a/drivers/scsi/nsp32.c b/drivers/scsi/nsp32.c
> index e893d56772417..9c9281222a0ac 100644
> --- a/drivers/scsi/nsp32.c
> +++ b/drivers/scsi/nsp32.c

[ ... ]

> @@ -3340,7 +3340,7 @@ static int nsp32_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	}
>  
>  	data->Pci	  = pdev;
> -	data->pci_devid   = id;
> +	data->model       = id->driver_data;

[Severity: High]
This is a pre-existing issue, but I have a question about how driver_data
is used.

If a user with root privileges writes a dynamic PCI ID to
/sys/bus/pci/drivers/nsp32/new_id with an arbitrary, large driver_data
value, the driver unconditionally uses this value as an index into the
7-element nsp32_model string array here in nsp32_probe() via
id->driver_data, and later in nsp32_show_info().

Could this unvalidated driver_data from dynamic PCI IDs cause an
out-of-bounds read?


[Severity: High]
This isn't a bug introduced by this patch, but I have a concern about
the global nsp32_data_base variable.

If two nsp32 PCI devices are probed concurrently, for example via concurrent
sysfs bind operations, both threads will concurrently modify the single
global static variable nsp32_data_base at the start of nsp32_probe() before
it is copied into the per-device host structure in nsp32_detect().

Does using this shared global variable for instance data create a race
condition in nsp32_probe()?


[Severity: Critical]
This is a pre-existing issue, but there might be an issue with the error
handling path.

If nsp32_detect() fails, it returns 1, and nsp32_probe() blindly propagates
this positive value to the PCI core. The PCI core treats positive return
values from probe() as a success, meaning the device successfully binds to
the driver even though initialization completely failed.

Because pci_set_drvdata() is not called on the error path, subsequent driver
callbacks like nsp32_remove() will receive a NULL pointer from
pci_get_drvdata(), causing a NULL pointer dereference.

Does nsp32_probe() incorrectly treat probe failures as success?

>  	data->IrqNumber   = pdev->irq;
>  	data->BaseAddress = pci_resource_start(pdev, 0);
>  	data->NumAddress  = pci_resource_len  (pdev, 0);

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but I noticed something in the
MMIO accessors.

In drivers/scsi/nsp32_io.h, nsp32_mmio_write4() explicitly uses cpu_to_le32()
in combination with writel(), and nsp32_mmio_read4() uses le32_to_cpu() with
readl(). Because writel() and readl() intrinsically perform the correct
host-to-PCI byte swapping, manually adding the byte-swap macros results in a
double byte-swap.

Do the MMIO read and write functions perform a double endianness swap on
big-endian architectures?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-pci_id_fix-v2-0-b834a98c0af2@garyguo.net?part=2

[PATCH v2 3/7] ipack: tpci200: don't keep pci_device_id

[Gary Guo]

pci_device_id is not guaranteed to live longer than probe due to presence
of dynamic ID. This stored ID is unused so remove it.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 drivers/ipack/carriers/tpci200.c | 1 -
 drivers/ipack/carriers/tpci200.h | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/ipack/carriers/tpci200.c b/drivers/ipack/carriers/tpci200.c
index 05dcb6675cd6..1cf51f763293 100644
--- a/drivers/ipack/carriers/tpci200.c
+++ b/drivers/ipack/carriers/tpci200.c
@@ -562,7 +562,6 @@ static int tpci200_pci_probe(struct pci_dev *pdev,
 
 	/* Save struct pci_dev pointer */
 	tpci200->info->pdev = pdev;
-	tpci200->info->id_table = (struct pci_device_id *)id;
 
 	/* register the device and initialize it */
 	ret = tpci200_install(tpci200);
diff --git a/drivers/ipack/carriers/tpci200.h b/drivers/ipack/carriers/tpci200.h
index e79ac64abcff..a2bf3125794b 100644
--- a/drivers/ipack/carriers/tpci200.h
+++ b/drivers/ipack/carriers/tpci200.h
@@ -145,7 +145,6 @@ struct tpci200_slot {
  */
 struct tpci200_infos {
 	struct pci_dev			*pdev;
-	struct pci_device_id		*id_table;
 	struct tpci200_regs __iomem	*interface_regs;
 	void __iomem			*cfg_regs;
 	struct ipack_bus_device		*ipack_bus;

-- 
2.54.0

Re: [PATCH v2 3/7] ipack: tpci200: don't keep pci_device_id

[Danilo Krummrich]

On Tue Jun 30, 2026 at 1:09 PM CEST, Gary Guo wrote:
> pci_device_id is not guaranteed to live longer than probe due to presence
> of dynamic ID. This stored ID is unused so remove it.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>

Reviewed-by: Danilo Krummrich <dakr@kernel.org>

Re: [PATCH v2 3/7] ipack: tpci200: don't keep pci_device_id

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] MMIO registers are accessed before the PCI device is enabled.
--

commit 30f524f5c4f44d8d3fe4dc97ba1c44684c0fe834
Author: Gary Guo <gary@garyguo.net>

ipack: tpci200: don't keep pci_device_id

This commit removes the unused pci_device_id table from the tpci200_infos
structure. The stored ID is unnecessary as dynamic IDs may not outlive
the probe function.

> diff --git a/drivers/ipack/carriers/tpci200.c b/drivers/ipack/carriers/tpci200.c
> index 05dcb6675cd66..1cf51f7632932 100644
> --- a/drivers/ipack/carriers/tpci200.c
> +++ b/drivers/ipack/carriers/tpci200.c
> @@ -562,7 +562,6 @@ static int tpci200_pci_probe(struct pci_dev *pdev,
>  
>  	/* Save struct pci_dev pointer */

[Severity: High]
This is a pre-existing issue, but does this probe function access
memory-mapped configuration registers before the PCI device is enabled?

Looking at tpci200_pci_probe() in drivers/ipack/carriers/tpci200.c,
we can see MMIO accesses happening just before this block:

drivers/ipack/carriers/tpci200.c:tpci200_pci_probe() {
	...
	reg32 = ioread32(tpci200->info->cfg_regs + LAS1_DESC);
	reg32 |= 1 << LAS_BIT_BIGENDIAN;
	iowrite32(reg32, tpci200->info->cfg_regs + LAS1_DESC);
	...
}

Since pci_enable_device() is not called until later down the call chain
in tpci200_register() (called via tpci200_install()), PCI_COMMAND_MEMORY
may not be set.

This can result in ignored writes, reading junk data (all 1s), and on
many architectures, it triggers a machine check exception, SError, or
kernel panic due to an Unsupported Request (UR) on the PCIe bus.

>  	tpci200->info->pdev = pdev;
> -	tpci200->info->id_table = (struct pci_device_id *)id;
>  
>  	/* register the device and initialize it */
>  	ret = tpci200_install(tpci200);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-pci_id_fix-v2-0-b834a98c0af2@garyguo.net?part=3

[PATCH v2 4/7] mlxsw: don't keep pci_device_id

[Gary Guo]

pci_device_id is not guaranteed to live longer than probe due to presence
of dynamic ID. This stored ID is unused so remove it.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
index 0da85d36647d..bfe3268dfdc1 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
@@ -130,7 +130,6 @@ struct mlxsw_pci {
 		} comp;
 	} cmd;
 	struct mlxsw_bus_info bus_info;
-	const struct pci_device_id *id;
 	enum mlxsw_pci_cqe_v max_cqe_ver; /* Maximal supported CQE version */
 	u8 num_cqs; /* Number of CQs */
 	u8 num_sdqs; /* Number of SDQs */
@@ -1768,7 +1767,6 @@ static void mlxsw_pci_mbox_free(struct mlxsw_pci *mlxsw_pci,
 }
 
 static int mlxsw_pci_sys_ready_wait(struct mlxsw_pci *mlxsw_pci,
-				    const struct pci_device_id *id,
 				    u32 *p_sys_status)
 {
 	unsigned long end;
@@ -1839,7 +1837,7 @@ static int mlxsw_pci_reset_sw(struct mlxsw_pci *mlxsw_pci)
 }
 
 static int
-mlxsw_pci_reset(struct mlxsw_pci *mlxsw_pci, const struct pci_device_id *id)
+mlxsw_pci_reset(struct mlxsw_pci *mlxsw_pci)
 {
 	struct pci_dev *pdev = mlxsw_pci->pdev;
 	bool pci_reset_sbr_supported = false;
@@ -1848,7 +1846,7 @@ mlxsw_pci_reset(struct mlxsw_pci *mlxsw_pci, const struct pci_device_id *id)
 	u32 sys_status;
 	int err;
 
-	err = mlxsw_pci_sys_ready_wait(mlxsw_pci, id, &sys_status);
+	err = mlxsw_pci_sys_ready_wait(mlxsw_pci, &sys_status);
 	if (err) {
 		dev_err(&pdev->dev, "Failed to reach system ready status before reset. Status is 0x%x\n",
 			sys_status);
@@ -1880,7 +1878,7 @@ mlxsw_pci_reset(struct mlxsw_pci *mlxsw_pci, const struct pci_device_id *id)
 	if (err)
 		return err;
 
-	err = mlxsw_pci_sys_ready_wait(mlxsw_pci, id, &sys_status);
+	err = mlxsw_pci_sys_ready_wait(mlxsw_pci, &sys_status);
 	if (err) {
 		dev_err(&pdev->dev, "Failed to reach system ready status after reset. Status is 0x%x\n",
 			sys_status);
@@ -1932,7 +1930,7 @@ static int mlxsw_pci_init(void *bus_priv, struct mlxsw_core *mlxsw_core,
 	if (!mbox)
 		return -ENOMEM;
 
-	err = mlxsw_pci_reset(mlxsw_pci, mlxsw_pci->id);
+	err = mlxsw_pci_reset(mlxsw_pci);
 	if (err)
 		goto err_reset;
 
@@ -2464,7 +2462,6 @@ static int mlxsw_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	mlxsw_pci->bus_info.device_name = pci_name(mlxsw_pci->pdev);
 	mlxsw_pci->bus_info.dev = &pdev->dev;
 	mlxsw_pci->bus_info.read_clock_capable = true;
-	mlxsw_pci->id = id;
 
 	err = mlxsw_core_bus_device_register(&mlxsw_pci->bus_info,
 					     &mlxsw_pci_bus, mlxsw_pci, false,

-- 
2.54.0

Re: [PATCH v2 4/7] mlxsw: don't keep pci_device_id

[Danilo Krummrich]

On Tue Jun 30, 2026 at 1:09 PM CEST, Gary Guo wrote:
> pci_device_id is not guaranteed to live longer than probe due to presence
> of dynamic ID. This stored ID is unused so remove it.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>

Reviewed-by: Danilo Krummrich <dakr@kernel.org>

Re: [PATCH v2 4/7] mlxsw: don't keep pci_device_id

[sashiko-bot]

> pci_device_id is not guaranteed to live longer than probe due to presence
> of dynamic ID. This stored ID is unused so remove it.
> 
> Signed-off-by: Gary Guo <gary@garyguo.net>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-pci_id_fix-v2-0-b834a98c0af2@garyguo.net?part=4

Re: [PATCH v2 4/7] mlxsw: don't keep pci_device_id

[Petr Machata]

Gary Guo <gary@garyguo.net> writes:

> pci_device_id is not guaranteed to live longer than probe due to presence
> of dynamic ID. This stored ID is unused so remove it.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>
> ---
>  drivers/net/ethernet/mellanox/mlxsw/pci.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
> index 0da85d36647d..bfe3268dfdc1 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c

> @@ -1768,7 +1767,6 @@ static void mlxsw_pci_mbox_free(struct mlxsw_pci *mlxsw_pci,
>  }
>  
>  static int mlxsw_pci_sys_ready_wait(struct mlxsw_pci *mlxsw_pci,
> -				    const struct pci_device_id *id,
>  				    u32 *p_sys_status)
>  {
>  	unsigned long end;

I see, we used this to detect whether we are on SwitchX-2. Support far
that was dropped ages ago in commit b0d80c013b04 ("mlxsw: Remove
Mellanox SwitchX-2 ASIC support").

Good cleanup, thanks.

Reviewed-by: Petr Machata <petrm@nvidia.com>

[PATCH v2 5/7] pci: make pci_match_one_device match on ID instead of device

[Gary Guo]

There is a need to match just IDs instead of against devices. Thus rename
this function to pci_match_one_id, and add a pci_id_from_device helper to
make it easy to convert users.

Similar convert pci_match_id to do_pci_match_id, however the existing API
is kept due to quite a few users.

Signed-off-by: Gary Guo <gary@garyguo.net>
---
 drivers/pci/pci-driver.c | 38 ++++++++++++++++++++++++++++----------
 drivers/pci/pci.h        | 36 ++++++++++++++++++++++++++----------
 drivers/pci/search.c     |  6 ++++--
 3 files changed, 58 insertions(+), 22 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index f36778e62ac1..0507cb801310 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -90,6 +90,27 @@ static void pci_free_dynids(struct pci_driver *drv)
 	spin_unlock(&drv->dynids.lock);
 }
 
+/**
+ * do_pci_match_id - See if a PCI ID matches a given pci_id table
+ * @ids: array of PCI device ID structures to search in
+ * @dev_id: the actual PCI device ID structure to match against.
+ *
+ * Returns the matching pci_device_id structure or
+ * %NULL if there is no match.
+ */
+static const struct pci_device_id *do_pci_match_id(const struct pci_device_id *ids,
+						   const struct pci_device_id *dev_id)
+{
+	if (ids) {
+		while (ids->vendor || ids->subvendor || ids->class_mask) {
+			if (pci_match_one_id(ids, dev_id))
+				return ids;
+			ids++;
+		}
+	}
+	return NULL;
+}
+
 /**
  * pci_match_id - See if a PCI device matches a given pci_id table
  * @ids: array of PCI device ID structures to search in
@@ -105,14 +126,9 @@ static void pci_free_dynids(struct pci_driver *drv)
 const struct pci_device_id *pci_match_id(const struct pci_device_id *ids,
 					 struct pci_dev *dev)
 {
-	if (ids) {
-		while (ids->vendor || ids->subvendor || ids->class_mask) {
-			if (pci_match_one_device(ids, dev))
-				return ids;
-			ids++;
-		}
-	}
-	return NULL;
+	struct pci_device_id dev_id = pci_id_from_device(dev);
+
+	return do_pci_match_id(ids, &dev_id);
 }
 EXPORT_SYMBOL(pci_match_id);
 
@@ -138,6 +154,7 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
 {
 	struct pci_dynid *dynid;
 	const struct pci_device_id *found_id = NULL, *ids;
+	struct pci_device_id dev_id;
 	int ret;
 
 	/* When driver_override is set, only bind to the matching driver */
@@ -145,10 +162,11 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
 	if (ret == 0)
 		return NULL;
 
+	dev_id = pci_id_from_device(dev);
 	/* Look at the dynamic ids first, before the static ones */
 	spin_lock(&drv->dynids.lock);
 	list_for_each_entry(dynid, &drv->dynids.list, node) {
-		if (pci_match_one_device(&dynid->id, dev)) {
+		if (pci_match_one_id(&dynid->id, &dev_id)) {
 			found_id = &dynid->id;
 			break;
 		}
@@ -158,7 +176,7 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
 	if (found_id)
 		return found_id;
 
-	for (ids = drv->id_table; (found_id = pci_match_id(ids, dev));
+	for (ids = drv->id_table; (found_id = do_pci_match_id(ids, &dev_id));
 	     ids = found_id + 1) {
 		/*
 		 * The match table is split based on driver_override.
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4469e1a77f3c..0567a8762baa 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -442,21 +442,37 @@ static inline int pci_setup_cardbus(char *str) { return -ENOENT; }
 #endif /* CONFIG_CARDBUS */
 
 /**
- * pci_match_one_device - Tell if a PCI device structure has a matching
- *			  PCI device id structure
- * @id: single PCI device id structure to match
- * @dev: the PCI device structure to match against
+ * pci_id_from_device - Obtain a pci_device_id from a PCI device
+ * @dev: the PCI device
+ *
+ * Returns a pci_device_id filled.
+ */
+static inline struct pci_device_id pci_id_from_device(const struct pci_dev *dev)
+{
+	return (struct pci_device_id) {
+		.vendor = dev->vendor,
+		.device = dev->device,
+		.subvendor = dev->subsystem_vendor,
+		.subdevice = dev->subsystem_device,
+		.class = dev->class,
+	};
+}
+
+/**
+ * pci_match_one_id - Tell if a PCI device ID matches a needle PCI device id
+ * @id: single PCI device id structure to match against (needle)
+ * @dev_id: the actual ID from the PCI device (can be created via pci_id_from_device)
  *
  * Returns the matching pci_device_id structure or %NULL if there is no match.
  */
 static inline const struct pci_device_id *
-pci_match_one_device(const struct pci_device_id *id, const struct pci_dev *dev)
+pci_match_one_id(const struct pci_device_id *id, const struct pci_device_id *dev_id)
 {
-	if ((id->vendor == PCI_ANY_ID || id->vendor == dev->vendor) &&
-	    (id->device == PCI_ANY_ID || id->device == dev->device) &&
-	    (id->subvendor == PCI_ANY_ID || id->subvendor == dev->subsystem_vendor) &&
-	    (id->subdevice == PCI_ANY_ID || id->subdevice == dev->subsystem_device) &&
-	    !((id->class ^ dev->class) & id->class_mask))
+	if ((id->vendor == PCI_ANY_ID || id->vendor == dev_id->vendor) &&
+	    (id->device == PCI_ANY_ID || id->device == dev_id->device) &&
+	    (id->subvendor == PCI_ANY_ID || id->subvendor == dev_id->subvendor) &&
+	    (id->subdevice == PCI_ANY_ID || id->subdevice == dev_id->subdevice) &&
+	    !((id->class ^ dev_id->class) & id->class_mask))
 		return id;
 	return NULL;
 }
diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index e3d3177fce54..c8c4bfe7817b 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -245,8 +245,10 @@ static int match_pci_dev_by_id(struct device *dev, const void *data)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	const struct pci_device_id *id = data;
+	struct pci_device_id dev_id;
 
-	if (pci_match_one_device(id, pdev))
+	dev_id = pci_id_from_device(pdev);
+	if (pci_match_one_id(id, &dev_id))
 		return 1;
 	return 0;
 }
@@ -418,7 +420,7 @@ EXPORT_SYMBOL(pci_get_class);
  *
  * Iterates through the list of known PCI devices. If a PCI device is found
  * with a matching base class code, the reference count to the device is
- * incremented. See pci_match_one_device() to figure out how does this works.
+ * incremented. See pci_match_one_id() to figure out how does this works.
  * A new search is initiated by passing %NULL as the @from argument.
  * Otherwise if @from is not %NULL, searches continue from next device on the
  * global list. The reference count for @from is always decremented if it is

-- 
2.54.0

Re: [PATCH v2 5/7] pci: make pci_match_one_device match on ID instead of device

[Danilo Krummrich]

On Tue Jun 30, 2026 at 1:09 PM CEST, Gary Guo wrote:
> There is a need to match just IDs instead of against devices. Thus rename
> this function to pci_match_one_id, and add a pci_id_from_device helper to
> make it easy to convert users.
>
> Similar convert pci_match_id to do_pci_match_id, however the existing API
> is kept due to quite a few users.
>
> Signed-off-by: Gary Guo <gary@garyguo.net>

Reviewed-by: Danilo Krummrich <dakr@kernel.org>

Re: [PATCH v2 5/7] pci: make pci_match_one_device match on ID instead of device

[sashiko-bot]

> There is a need to match just IDs instead of against devices. Thus rename
> this function to pci_match_one_id, and add a pci_id_from_device helper to
> make it easy to convert users.
> 
> Similar convert pci_match_id to do_pci_match_id, however the existing API
> is kept due to quite a few users.
> 
> Signed-off-by: Gary Guo <gary@garyguo.net>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-pci_id_fix-v2-0-b834a98c0af2@garyguo.net?part=5

[PATCH v2 6/7] pci: fix dyn_id add TOCTOU

[Gary Guo]

Currently there is a TOCTOU issue in new_id_store as the dyn ID insertion
in pci_add_dynid and the pci_match_device are in separate critical
sections.

Fix this by moving the existing ID check to inside pci_add_dynid and only
check against the static ID table outside the critical section.

Fixes: 3853f9123c18 ("PCI: Avoid duplicate IDs in driver dynamic IDs list")
Signed-off-by: Gary Guo <gary@garyguo.net>
---
 drivers/pci/pci-driver.c | 139 ++++++++++++++++++++++++-----------------------
 1 file changed, 71 insertions(+), 68 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 0507cb801310..df1be7ea2bde 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -29,6 +29,48 @@ struct pci_dynid {
 	struct pci_device_id id;
 };
 
+/**
+ * do_pci_add_dynid - add a new PCI device ID to this driver and re-probe devices
+ * @drv: target pci driver
+ * @id: ID to be added
+ * @check_dup: whether to check if matching ID is already present
+ *
+ * Adds a new dynamic pci device ID to this driver and causes the
+ * driver to probe for all devices again.  @drv must have been
+ * registered prior to calling this function.
+ *
+ * CONTEXT:
+ * Does GFP_KERNEL allocation.
+ *
+ * RETURNS:
+ * 0 on success, -errno on failure.
+ */
+static int do_pci_add_dynid(struct pci_driver *drv, const struct pci_device_id *id, bool check_dup)
+{
+	struct pci_dynid *dynid, *existing_dynid;
+
+	dynid = kzalloc_obj(*dynid);
+	if (!dynid)
+		return -ENOMEM;
+
+	dynid->id = *id;
+
+	{
+		guard(spinlock)(&drv->dynids.lock);
+		if (check_dup) {
+			list_for_each_entry(existing_dynid, &drv->dynids.list, node) {
+				if (pci_match_one_id(&existing_dynid->id, id)) {
+					kfree(dynid);
+					return -EEXIST;
+				}
+			}
+		}
+		list_add_tail(&dynid->node, &drv->dynids.list);
+	}
+
+	return driver_attach(&drv->driver);
+}
+
 /**
  * pci_add_dynid - add a new PCI device ID to this driver and re-probe devices
  * @drv: target pci driver
@@ -56,25 +98,17 @@ int pci_add_dynid(struct pci_driver *drv,
 		  unsigned int class, unsigned int class_mask,
 		  unsigned long driver_data)
 {
-	struct pci_dynid *dynid;
+	struct pci_device_id id = {
+		.vendor = vendor,
+		.device = device,
+		.subvendor = subvendor,
+		.subdevice = subdevice,
+		.class = class,
+		.class_mask = class_mask,
+		.driver_data = driver_data,
+	};
 
-	dynid = kzalloc_obj(*dynid);
-	if (!dynid)
-		return -ENOMEM;
-
-	dynid->id.vendor = vendor;
-	dynid->id.device = device;
-	dynid->id.subvendor = subvendor;
-	dynid->id.subdevice = subdevice;
-	dynid->id.class = class;
-	dynid->id.class_mask = class_mask;
-	dynid->id.driver_data = driver_data;
-
-	spin_lock(&drv->dynids.lock);
-	list_add_tail(&dynid->node, &drv->dynids.list);
-	spin_unlock(&drv->dynids.lock);
-
-	return driver_attach(&drv->driver);
+	return do_pci_add_dynid(drv, &id, false);
 }
 EXPORT_SYMBOL_GPL(pci_add_dynid);
 
@@ -99,11 +133,13 @@ static void pci_free_dynids(struct pci_driver *drv)
  * %NULL if there is no match.
  */
 static const struct pci_device_id *do_pci_match_id(const struct pci_device_id *ids,
-						   const struct pci_device_id *dev_id)
+						   const struct pci_device_id *dev_id,
+						   bool match_override_only)
 {
 	if (ids) {
 		while (ids->vendor || ids->subvendor || ids->class_mask) {
-			if (pci_match_one_id(ids, dev_id))
+			if ((!ids->override_only || match_override_only) &&
+			    pci_match_one_id(ids, dev_id))
 				return ids;
 			ids++;
 		}
@@ -128,7 +164,7 @@ const struct pci_device_id *pci_match_id(const struct pci_device_id *ids,
 {
 	struct pci_device_id dev_id = pci_id_from_device(dev);
 
-	return do_pci_match_id(ids, &dev_id);
+	return do_pci_match_id(ids, &dev_id, true);
 }
 EXPORT_SYMBOL(pci_match_id);
 
@@ -153,7 +189,7 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
 						    struct pci_dev *dev)
 {
 	struct pci_dynid *dynid;
-	const struct pci_device_id *found_id = NULL, *ids;
+	const struct pci_device_id *found_id = NULL;
 	struct pci_device_id dev_id;
 	int ret;
 
@@ -176,20 +212,9 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
 	if (found_id)
 		return found_id;
 
-	for (ids = drv->id_table; (found_id = do_pci_match_id(ids, &dev_id));
-	     ids = found_id + 1) {
-		/*
-		 * The match table is split based on driver_override.
-		 * In case override_only was set, enforce driver_override
-		 * matching.
-		 */
-		if (found_id->override_only) {
-			if (ret > 0)
-				return found_id;
-		} else {
-			return found_id;
-		}
-	}
+	found_id = do_pci_match_id(drv->id_table, &dev_id, ret > 0);
+	if (found_id)
+		return found_id;
 
 	/* driver_override will always match, send a dummy id */
 	if (ret > 0)
@@ -197,11 +222,6 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
 	return NULL;
 }
 
-static void _pci_free_device(struct device *dev)
-{
-	kfree(to_pci_dev(dev));
-}
-
 /**
  * new_id_store - sysfs frontend to pci_add_dynid()
  * @driver: target device driver
@@ -215,38 +235,22 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf,
 {
 	struct pci_driver *pdrv = to_pci_driver(driver);
 	const struct pci_device_id *ids = pdrv->id_table;
-	u32 vendor, device, subvendor = PCI_ANY_ID,
-		subdevice = PCI_ANY_ID, class = 0, class_mask = 0;
-	unsigned long driver_data = 0;
+	struct pci_device_id id = {
+		.subvendor = PCI_ANY_ID,
+		.subdevice = PCI_ANY_ID
+	};
 	int fields;
 	int retval = 0;
 
 	fields = sscanf(buf, "%x %x %x %x %x %x %lx",
-			&vendor, &device, &subvendor, &subdevice,
-			&class, &class_mask, &driver_data);
+			&id.vendor, &id.device, &id.subvendor, &id.subdevice,
+			&id.class, &id.class_mask, &id.driver_data);
 	if (fields < 2)
 		return -EINVAL;
 
 	if (fields != 7) {
-		struct pci_dev *pdev = kzalloc_obj(*pdev);
-		if (!pdev)
-			return -ENOMEM;
-
-		pdev->vendor = vendor;
-		pdev->device = device;
-		pdev->subsystem_vendor = subvendor;
-		pdev->subsystem_device = subdevice;
-		pdev->class = class;
-		pdev->dev.release = _pci_free_device;
-
-		device_initialize(&pdev->dev);
-		if (pci_match_device(pdrv, pdev))
-			retval = -EEXIST;
-
-		put_device(&pdev->dev);
-
-		if (retval)
-			return retval;
+		if (do_pci_match_id(pdrv->id_table, &id, false))
+			return -EEXIST;
 	}
 
 	/* Only accept driver_data values that match an existing id_table
@@ -254,7 +258,7 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf,
 	if (ids) {
 		retval = -EINVAL;
 		while (ids->vendor || ids->subvendor || ids->class_mask) {
-			if (driver_data == ids->driver_data) {
+			if (id.driver_data == ids->driver_data) {
 				retval = 0;
 				break;
 			}
@@ -264,8 +268,7 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf,
 			return retval;
 	}
 
-	retval = pci_add_dynid(pdrv, vendor, device, subvendor, subdevice,
-			       class, class_mask, driver_data);
+	retval = do_pci_add_dynid(pdrv, &id, fields != 7);
 	if (retval)
 		return retval;
 	return count;

-- 
2.54.0

Re: [PATCH v2 6/7] pci: fix dyn_id add TOCTOU

[Danilo Krummrich]

On Tue Jun 30, 2026 at 1:09 PM CEST, Gary Guo wrote:
> +static int do_pci_add_dynid(struct pci_driver *drv, const struct pci_device_id *id, bool check_dup)
> +{
> +	struct pci_dynid *dynid, *existing_dynid;
> +
> +	dynid = kzalloc_obj(*dynid);
> +	if (!dynid)
> +		return -ENOMEM;
> +
> +	dynid->id = *id;
> +
> +	{
> +		guard(spinlock)(&drv->dynids.lock);
> +		if (check_dup) {
> +			list_for_each_entry(existing_dynid, &drv->dynids.list, node) {
> +				if (pci_match_one_id(&existing_dynid->id, id)) {
> +					kfree(dynid);
> +					return -EEXIST;
> +				}
> +			}
> +		}
> +		list_add_tail(&dynid->node, &drv->dynids.list);
> +	}

This should use scoped_guard(spinlock, &drv->dynids.lock) instead.

>  static const struct pci_device_id *do_pci_match_id(const struct pci_device_id *ids,
> -						   const struct pci_device_id *dev_id)
> +						   const struct pci_device_id *dev_id,
> +						   bool match_override_only)

Maybe something along the lines of include_override_only? At a quick glance
match_override_only could be read as "match override-only entries exclusively".

Re: [PATCH v2 6/7] pci: fix dyn_id add TOCTOU

[sashiko-bot]

> Currently there is a TOCTOU issue in new_id_store as the dyn ID insertion
> in pci_add_dynid and the pci_match_device are in separate critical
> sections.
> 
> Fix this by moving the existing ID check to inside pci_add_dynid and only
> check against the static ID table outside the critical section.
> 
> Fixes: 3853f9123c18 ("PCI: Avoid duplicate IDs in driver dynamic IDs list")
> Signed-off-by: Gary Guo <gary@garyguo.net>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-pci_id_fix-v2-0-b834a98c0af2@garyguo.net?part=6

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

[Gary Guo]

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. Also, clarify that the id parameter is only valid during
probe.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/all/20260619170503.518F61F00A3A@smtp.kernel.org/
Fixes: 0994375e9614 ("PCI: add remove_id sysfs entry")
Signed-off-by: Gary Guo <gary@garyguo.net>
---
 drivers/pci/pci-driver.c | 58 ++++++++++++++++++++++++------------------------
 include/linux/pci.h      |  1 +
 2 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index df1be7ea2bde..fad028b9dc53 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -179,14 +179,16 @@ static const struct pci_device_id pci_device_id_any = {
  * pci_match_device - See if a device matches a driver's list of IDs
  * @drv: the PCI driver to match against
  * @dev: the PCI device structure to match against
+ * @id: Matched pci_device_id
  *
  * Used by a driver to check whether a PCI device is in its list of
  * supported devices or in the dynids list, which may have been augmented
- * via the sysfs "new_id" file.  Returns the matching pci_device_id
- * structure or %NULL if there is no match.
+ * via the sysfs "new_id" file.  Returns true if there is a match, the matched
+ * ID is stored in @id.
  */
-static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
-						    struct pci_dev *dev)
+static bool pci_match_device(struct pci_driver *drv,
+			     struct pci_dev *dev,
+			     struct pci_device_id *id)
 {
 	struct pci_dynid *dynid;
 	const struct pci_device_id *found_id = NULL;
@@ -196,30 +198,33 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
 	/* When driver_override is set, only bind to the matching driver */
 	ret = device_match_driver_override(&dev->dev, &drv->driver);
 	if (ret == 0)
-		return NULL;
+		return false;
 
 	dev_id = pci_id_from_device(dev);
 	/* Look at the dynamic ids first, before the static ones */
-	spin_lock(&drv->dynids.lock);
-	list_for_each_entry(dynid, &drv->dynids.list, node) {
-		if (pci_match_one_id(&dynid->id, &dev_id)) {
-			found_id = &dynid->id;
-			break;
+	{
+		guard(spinlock)(&drv->dynids.lock);
+		list_for_each_entry(dynid, &drv->dynids.list, node) {
+			if (pci_match_one_id(&dynid->id, &dev_id)) {
+				*id = dynid->id;
+				return true;
+			}
 		}
 	}
-	spin_unlock(&drv->dynids.lock);
-
-	if (found_id)
-		return found_id;
 
 	found_id = do_pci_match_id(drv->id_table, &dev_id, ret > 0);
-	if (found_id)
-		return found_id;
+	if (found_id) {
+		*id = *found_id;
+		return true;
+	}
 
 	/* driver_override will always match, send a dummy id */
-	if (ret > 0)
-		return &pci_device_id_any;
-	return NULL;
+	if (ret > 0) {
+		*id = pci_device_id_any;
+		return true;
+	}
+
+	return false;
 }
 
 /**
@@ -465,15 +470,14 @@ void pci_probe_flush_workqueue(void)
  */
 static int __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)
 {
-	const struct pci_device_id *id;
+	struct pci_device_id id;
 	int error = 0;
 
 	if (drv->probe) {
 		error = -ENODEV;
 
-		id = pci_match_device(drv, pci_dev);
-		if (id)
-			error = pci_call_probe(drv, pci_dev, id);
+		if (pci_match_device(drv, pci_dev, &id))
+			error = pci_call_probe(drv, pci_dev, &id);
 	}
 	return error;
 }
@@ -1558,17 +1562,13 @@ static int pci_bus_match(struct device *dev, const struct device_driver *drv)
 {
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	struct pci_driver *pci_drv;
-	const struct pci_device_id *found_id;
+	struct pci_device_id id;
 
 	if (pci_dev_binding_disallowed(pci_dev))
 		return 0;
 
 	pci_drv = (struct pci_driver *)to_pci_driver(drv);
-	found_id = pci_match_device(pci_drv, pci_dev);
-	if (found_id)
-		return 1;
-
-	return 0;
+	return pci_match_device(pci_drv, pci_dev, &id);
 }
 
 /**
diff --git a/include/linux/pci.h b/include/linux/pci.h
index ebb5b9d76360..f128d8c0cbb6 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -979,6 +979,7 @@ struct module;
  *		function returns zero when the driver chooses to
  *		take "ownership" of the device or an error code
  *		(negative number) otherwise.
+ *		The pci_device_id parameter is only valid during probe.
  *		The probe function always gets called from process
  *		context, so it can sleep.
  * @remove:	The remove() function gets called whenever a device

-- 
2.54.0

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

[Danilo Krummrich]

On Tue Jun 30, 2026 at 1:09 PM CEST, Gary Guo wrote:
> -static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
> -						    struct pci_dev *dev)
> +static bool pci_match_device(struct pci_driver *drv,
> +			     struct pci_dev *dev,
> +			     struct pci_device_id *id)
>  {
>  	struct pci_dynid *dynid;
>  	const struct pci_device_id *found_id = NULL;
> @@ -196,30 +198,33 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
>  	/* When driver_override is set, only bind to the matching driver */
>  	ret = device_match_driver_override(&dev->dev, &drv->driver);
>  	if (ret == 0)
> -		return NULL;
> +		return false;
>  
>  	dev_id = pci_id_from_device(dev);
>  	/* Look at the dynamic ids first, before the static ones */
> -	spin_lock(&drv->dynids.lock);
> -	list_for_each_entry(dynid, &drv->dynids.list, node) {
> -		if (pci_match_one_id(&dynid->id, &dev_id)) {
> -			found_id = &dynid->id;
> -			break;
> +	{
> +		guard(spinlock)(&drv->dynids.lock);
> +		list_for_each_entry(dynid, &drv->dynids.list, node) {
> +			if (pci_match_one_id(&dynid->id, &dev_id)) {
> +				*id = dynid->id;
> +				return true;
> +			}
>  		}
>  	}

Should be scoped_guard(spinlock, &drv->dynids.lock). It also looks like dynid
could be moved into the scoped_guard().

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `id` parameter passed to PCI driver probe functions is now a pointer to a stack-allocated variable, breaking drivers that rely on pointer arithmetic to calculate indices or that cache the pointer.
--

commit 0cf7c75b08e81435f8eec9e2453a6dda8624a99b
Author: Gary Guo <gary@garyguo.net>
Subject: pci: fix UAF when probe runs concurrent to dyn ID removal

This commit modifies the PCI matching logic to copy the matched device ID
instead of returning a pointer to it. This ensures dynamic IDs are safely
handled without requiring the lock to be held during probe.

Link: https://lore.kernel.org/all/20260619170503.518F61F00A3A@smtp.kernel.org/

> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c

[ ... ]

> @@ -465,15 +470,14 @@ void pci_probe_flush_workqueue(void)
>   */
>  static int __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)
>  {
> -	const struct pci_device_id *id;
> +	struct pci_device_id id;
>  	int error = 0;
>  
>  	if (drv->probe) {
>  		error = -ENODEV;
>  
> -		id = pci_match_device(drv, pci_dev);
> -		if (id)
> -			error = pci_call_probe(drv, pci_dev, id);
> +		if (pci_match_device(drv, pci_dev, &id))
> +			error = pci_call_probe(drv, pci_dev, &id);

[Severity: High]
Does passing a pointer to the stack-allocated id variable here break drivers
that rely on the pointer belonging to their static id_table?

Some drivers compute an array index using pointer arithmetic against the
provided id pointer. For example, in drivers/char/agp/amd-k7-agp.c:

agp_amdk7_probe() {
    ...
    j = ent - agp_amdk7_pci_table;
    dev_info(&pdev->dev, "AMD %s chipset\n",
             amd_agp_device_ids[j].chipset_name);
    ...
}

Since the pointer now points to the stack frame in __pci_device_probe()
rather than the static table, won't this pointer subtraction yield a
wildly out-of-bounds index and cause a memory read fault during device
probing?

>  	}
>  	return error;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-pci_id_fix-v2-0-b834a98c0af2@garyguo.net?part=7

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

[Gary Guo]

On Wed Jul 1, 2026 at 12:10 PM BST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The `id` parameter passed to PCI driver probe functions is now a pointer to a stack-allocated variable, breaking drivers that rely on pointer arithmetic to calculate indices or that cache the pointer.
> --
>
> commit 0cf7c75b08e81435f8eec9e2453a6dda8624a99b
> Author: Gary Guo <gary@garyguo.net>
> Subject: pci: fix UAF when probe runs concurrent to dyn ID removal
>
> This commit modifies the PCI matching logic to copy the matched device ID
> instead of returning a pointer to it. This ensures dynamic IDs are safely
> handled without requiring the lock to be held during probe.
>
> Link: https://lore.kernel.org/all/20260619170503.518F61F00A3A@smtp.kernel.org/
>
>> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
>> --- a/drivers/pci/pci-driver.c
>> +++ b/drivers/pci/pci-driver.c
>
> [ ... ]
>
>> @@ -465,15 +470,14 @@ void pci_probe_flush_workqueue(void)
>>   */
>>  static int __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)
>>  {
>> -	const struct pci_device_id *id;
>> +	struct pci_device_id id;
>>  	int error = 0;
>>  
>>  	if (drv->probe) {
>>  		error = -ENODEV;
>>  
>> -		id = pci_match_device(drv, pci_dev);
>> -		if (id)
>> -			error = pci_call_probe(drv, pci_dev, id);
>> +		if (pci_match_device(drv, pci_dev, &id))
>> +			error = pci_call_probe(drv, pci_dev, &id);
>
> [Severity: High]
> Does passing a pointer to the stack-allocated id variable here break drivers
> that rely on the pointer belonging to their static id_table?
>
> Some drivers compute an array index using pointer arithmetic against the
> provided id pointer. For example, in drivers/char/agp/amd-k7-agp.c:
>
> agp_amdk7_probe() {
>     ...
>     j = ent - agp_amdk7_pci_table;
>     dev_info(&pdev->dev, "AMD %s chipset\n",
>              amd_agp_device_ids[j].chipset_name);
>     ...
> }

I am amused both by creative ways that drivers make use of this ID (which, btw,
is completely wrong without this change already and would be OOB access for any
dynamic ID or driver_override) and by the AI's ability to find them.

I'm now convinced that the drivers shouldn't be given the pci_device_id in the
first place but just the driver_data field, although it's going to be very
difficult to change all the drivers so that might never happen.

Best,
Gary

>
> Since the pointer now points to the stack frame in __pci_device_probe()
> rather than the static table, won't this pointer subtraction yield a
> wildly out-of-bounds index and cause a memory read fault during device
> probing?
>
>>  	}
>>  	return error;
>>  }