Re: [PATCH v3 1/2] PCI: Ignore Immediate Readiness on Samsung 990 PRO NVMe

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pci: read-modify-write data race on dev->imm_ready bitfield
--

commit 0139f0dcf6cc99c5e4527404461aefca31dd96e8
Author: Alexander Gruhlke <gruhlke@mailbox.org>

PCI: Ignore Immediate Readiness on Samsung 990 PRO NVMe

The Samsung 990 PRO advertises Immediate Readiness, so pcie_flr() doesn't
wait after FLR, but the drive returns ~0 for up to 7 ms. Ignore Immediate
Readiness on this device, so pcie_flr() waits.

Link: https://bbs.archlinux.org/viewtopic.php?id=314833
Link: https://lore.kernel.org/r/20261010160529.27974-1-gruhlke@mailbox.org

> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 38af25c390867..fc61e1ad9e574 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -5747,6 +5747,17 @@ static void quirk_no_flr_snet(struct pci_dev *dev)
>  }
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SOLIDRUN, 0x1000, quirk_no_flr_snet);
>  
> +/* Samsung 990 PRO advertises Immediate Readiness, but isn't ready after FLR */
> +static void quirk_no_imm_ready(struct pci_dev *dev)
> +{
> +	if (!dev->imm_ready)
> +		return;
> +
> +	pci_info(dev, "Ignoring incorrectly advertised Immediate Readiness support\n");
> +	dev->imm_ready = 0;

[Severity: Medium]
Could this bitfield update be subject to a read-modify-write data race with
sysfs stores?

Because this quirk is executed via DECLARE_PCI_FIXUP_FINAL, it is invoked by
pci_bus_add_device(). This happens after pci_device_add() has already called
device_add(), exposing the device's sysfs attributes and firing a KOBJ_ADD
uevent.

This leaves a window where userspace (such as udev) might concurrently
write to sysfs attributes. Sysfs store handlers like d3cold_allowed_store()
write to bitfields that share the same underlying word as imm_ready without
locking.

Can this result in a C bitfield data race where either the imm_ready = 0
update or the sysfs attribute update is silently lost, potentially breaking
the VFIO passthrough (the bug this patch intended to fix) or corrupting PCI
power management state?

> +}
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_SAMSUNG, 0xa80c, quirk_no_imm_ready);
> +
>  static void quirk_no_ext_tags(struct pci_dev *pdev)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011190145.23621-1-gruhlke@mailbox.org?part=1