All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Gary Guo" <gary@garyguo.net>
Cc: linux-scsi@vger.kernel.org, dlemoal@kernel.org,
	cassel@kernel.org, linux-ide@vger.kernel.org,
	linux-pci@vger.kernel.org
Subject: Re: [PATCH v2 7/7] pci: fix UAF when probe runs concurrent to dyn ID removal
Date: Wed, 01 Jul 2026 11:10:51 +0000	[thread overview]
Message-ID: <20260701111052.3E04E1F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260630-pci_id_fix-v2-7-b834a98c0af2@garyguo.net>

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

  parent reply	other threads:[~2026-07-01 11:10 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-30 11:09 [PATCH v2 0/7] pci: fix UAF and TOCTOU related to dynamic ID Gary Guo
2026-06-30 11:09 ` [PATCH v2 1/7] ata: don't keep pci_device_id Gary Guo
2026-06-30 11:59   ` Niklas Cassel
2026-06-30 12:41     ` Gary Guo
2026-06-30 19:46   ` Danilo Krummrich
2026-07-01 11:10   ` sashiko-bot
2026-06-30 11:09 ` [PATCH v2 2/7] nsp32: " Gary Guo
2026-06-30 19:46   ` Danilo Krummrich
2026-07-01 11:10   ` sashiko-bot
2026-06-30 11:09 ` [PATCH v2 3/7] ipack: tpci200: " Gary Guo
2026-06-30 19:47   ` Danilo Krummrich
2026-07-01 11:10   ` sashiko-bot
2026-06-30 11:09 ` [PATCH v2 4/7] mlxsw: " Gary Guo
2026-06-30 19:48   ` Danilo Krummrich
2026-07-01 11:10   ` sashiko-bot
2026-07-01 13:57   ` Petr Machata
2026-06-30 11:09 ` [PATCH v2 5/7] pci: make pci_match_one_device match on ID instead of device Gary Guo
2026-06-30 20:04   ` Danilo Krummrich
2026-07-01 11:10   ` sashiko-bot
2026-06-30 11:09 ` [PATCH v2 6/7] pci: fix dyn_id add TOCTOU Gary Guo
2026-06-30 20:16   ` Danilo Krummrich
2026-07-01 11:10   ` sashiko-bot
2026-06-30 11:09 ` [PATCH v2 7/7] pci: fix UAF when probe runs concurrent to dyn ID removal Gary Guo
2026-06-30 20:25   ` Danilo Krummrich
2026-07-01 11:10   ` sashiko-bot [this message]
2026-07-01 11:44     ` Gary Guo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260701111052.3E04E1F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=cassel@kernel.org \
    --cc=dlemoal@kernel.org \
    --cc=gary@garyguo.net \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.