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

Bjorn Helgaas <[email protected]> Tue, 21 Jul 2026 17:30:47 -0500
Newsgroups gmane.linux.ide,gmane.linux.kernel.pci,gmane.linux.kernel,gmane.linux.scsi,gmane.linux.kernel.ipack,gmane.linux.network
Message-ID <20260721223047.GA674520@bhelgaas>
On Tue, Jun 30, 2026 at 12:09:00PM +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. 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/[email protected]/ [1]
> Link: https://lore.kernel.org/all/[email protected]/ [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/[email protected]
> 
> ---
> 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

Run "git log --oneline drivers/pci" and match style (capitalize "PCI:"
and first word of subject).

At least in the PCI patch commit logs and comments, add "()" after
function names.

s/id/ID/ throughout (it's currently inconsistent).
s/pci/PCI/ throughout (ditto)