Re: [RESEND PATCH] PCI: Fix race condition between block_cfg_access and msi_enabled/msix_enabled

[Yicong Yang <yangyccccc@gmail.com>]

On 2026/3/16 17:56, David Woodhouse wrote:
>> Previously we use bit field for block_cfg_access and
>> msi_enabled/msix_enabled, which is non-atomic and they may race
>> each other as they share the same memory region. A race condition
>> is met between driver bind vs FLR through sysfs:
>>
>> for driver bind side in thread 1:
>> ...
>> device_lock()
>> ...
>>   ->probe()
>>     pci_alloc_irq_vectors_affinity()
>>       __pci_enable_msi_range()
>>         msi_capability_init()
>>           dev->msi_enabled=1 <---set here
>>     request_irq(pci_irq_vector(),...)
>>
>> when echo 1 > reset in thread 2:
>> pci_reset_function()
>>   pci_dev_lock()
>>     pci_cfg_access_lock()
>>       dev->block_cfg_access=1 <---may overwrite msi_enabled bit
>>     device_lock()
>>
>> The msi_enabled bit may be overwritten to 0 and will trigger the WARN
>> assert in pci_irq_vector(). A similar issue has been addressed in
>> commit 44bda4b7d26e ("PCI: Fix is_added/is_busmaster race
>> condition").
>>
>> Move the block_cfg_access to the pci_dev->priv_flags and use atomic
>> bit operations to avoid the race condition.
>>
>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> Looks like you dropped this... since commit c9a81f9ed6a ("PCI: Avoid
> pci_dev_lock() AB/BA deadlock with sriov_numvfs_store()") we now take
> the device lock *before* pci_cfg_access_lock(), so the second thread of
> your race description above can no longer happen, right?

yes you're right. now dev->block_cfg_access is protected by device_lock() and will not race with
thread 1, as dev->block_cfg_access is also protected by device_lock().