Re: [PATCH AUTOSEL 6.18-5.10] PCI/sysfs: Use kstrtobool() to parse the ROM attribute input

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unlocked read-modify-write of the `rom_attr_enabled` bitfield in `struct pci_dev` exposes a data race.
--

commit ede1ff2652bff89ab4ad252e8e7e79bb533b4702
Author: Krzysztof Wilczyński <kwilczynski@kernel.org>
PCI/sysfs: Use kstrtobool() to parse the ROM attribute input

This patch modifies pci_write_rom() to parse sysfs input using kstrtobool().
It ensures that the ROM attribute is properly disabled even when a trailing
newline is missing from the input, and it rejects invalid inputs with -EINVAL.

Link: https://patch.msgid.link/20260612182448.552406-1-kwilczynski@kernel.org

> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1336,18 +1336,19 @@ void __weak pci_remove_resource_files(struct pci_dev *dev) { return; }
[ ... ]
>  static ssize_t pci_write_rom(struct file *filp, struct kobject *kobj,
>  			     const struct bin_attribute *bin_attr, char *buf,
>  			     loff_t off, size_t count)
>  {
>  	struct pci_dev *pdev = to_pci_dev(kobj_to_dev(kobj));
> +	bool enable;
>  
> -	if ((off ==  0) && (*buf == '0') && (count == 2))
> -		pdev->rom_attr_enabled = 0;
> -	else
> -		pdev->rom_attr_enabled = 1;
> +	if (kstrtobool(buf, &enable))
> +		return -EINVAL;
> +
> +	pdev->rom_attr_enabled = enable;

[Severity: High]
This is a pre-existing issue, but does this unlocked assignment expose a
data race?

In include/linux/pci.h, rom_attr_enabled is declared as a bitfield:

    struct pci_dev {
        ...
        unsigned int rom_attr_enabled:1;
        ...
    };

This bitfield shares a 32-bit machine word with other dynamically updated
fields like is_probed and irq_managed.

Since C bitfield assignments compile to non-atomic read-modify-write
operations on the underlying machine word, could a concurrent write to the
rom sysfs attribute race with updates to these other bitfields?

If a privileged user writes to the rom sysfs attribute concurrently with the
device being probed (where is_probed is toggled) or its IRQs being
configured, could this silently corrupt the word and result in deadlocks in
VF probing or IRQ resource leaks?

>  
>  	return count;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-534-sashal@kernel.org?part=1