Re: [PATCH 2/7] vfio: Add qcom_vfio.h header for MSI cache protocol

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] vfio: protocol abuses hardware config space risking data corruption
- [High] vfio: blind config space scan causes URs and read side-effects
- [High] vfio: false positive MSI discovery from arbitrary config data
- [Medium] vfio: incomplete MSI cache search bounds silently fails
--

commit e548d5bbb36a1137ddec4454b7d63b995f23989e
Author: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>

vfio: Add qcom_vfio.h header for MSI cache protocol

Add shared header defining the communication protocol between
qcom-vfio-pci variant driver and Qualcomm device drivers (ath11k,
ath12k) for passing physical host MSI addresses to VMs.

Qualcomm device firmware requires physical host MSI addresses rather
than guest IOVA addresses. The qcom-vfio-pci driver caches host MSI
values in extended PCI config space with a magic signature "QMSI" for
VM discovery.

> diff --git a/include/linux/qcom_vfio.h b/include/linux/qcom_vfio.h
> new file mode 100644
> index 0000000000000..2ed1ea0c1a604
> --- /dev/null
> +++ b/include/linux/qcom_vfio.h
[ ... ]
> +static inline int qcom_vfio_find_msi_offset(struct pci_dev *pdev)
> +{
> +	return pci_find_free_ext_cap_offset(pdev, QCOM_VFIO_MSI_SPACE_SIZE);
> +}

[Severity: Critical]
Does using pci_find_free_ext_cap_offset() here risk corrupting active
hardware capabilities?

pci_find_free_ext_cap_offset() relies on a heuristic that assumes the last
extended capability is exactly 64 bytes:

drivers/pci/pci.c:pci_find_free_ext_cap_offset() {
    ...
    free_offset = last_cap_start + 64;
    free_offset = ALIGN(free_offset, 4);
    ...
}

If the final capability is larger (such as AER, DPC, or VSEC), this
allocation would return an offset inside the active capability. Will the
host driver physically writing MSI data to this offset silently corrupt the
device's hardware capability state?

> +
> +/**
> + * qcom_vfio_find_msi_cache - Locate cached MSI data by magic signature
> + * @pdev: PCI device
> + *
> + * VM-side function: Searches extended config space for "QMSI" magic signature
> + * to locate host-cached MSI values. This avoids offset mismatch when QEMU
> + * filters certain capabilities.
> + *
> + * Returns: Offset of MSI data (after magic), or 0 if not found
> + */
> +static inline int qcom_vfio_find_msi_cache(struct pci_dev *pdev)
> +{
> +	u32 magic;
> +	int offset;
> +
> +	for (offset = 0x100; offset <= 0xf00; offset += 4) {

[Severity: Medium]
Will this loop prematurely terminate and fail to find the MSI cache if the
offset is allocated above 0xf00?

The host driver uses pci_find_free_ext_cap_offset(), which can legitimately
return an offset up to 0xff0 in the 4096-byte PCIe extended config space.
If the cache block is placed between 0xf04 and 0xff0, the guest driver
would silently fail to discover the magic signature.

> +		if (pci_read_config_dword(pdev, offset, &magic))
> +			continue;

[Severity: High]
Does blindly reading every DWORD in the extended config space risk bus
errors and unintended hardware side effects?

Iterating through the configuration space without walking the PCIe
capability linked list means reading active capability registers
arbitrarily. This can trigger unintended read side-effects, such as
clearing Read-to-Clear (RC) status bits.

Furthermore, reading unmapped PCIe configuration space can generate
Unsupported Request (UR) bus errors. On some systems, URs escalate to
Advanced Error Reporting (AER) faults or system hangs.

> +		if (magic == QCOM_VFIO_MSI_MAGIC)
> +			return offset;

[Severity: High]
Could this lead to false positive MSI discoveries if active registers or
payloads contain this value?

Since the loop scans the entire config space instead of parsing
vendor-specific capabilities, a legitimate device register, statistics
counter, or VSEC payload might naturally contain the 32-bit value
0x49534D51 (QMSI).

If falsely identified, the guest driver would interpret the subsequent 12
bytes as physical MSI addresses and mistakenly program invalid addresses
into the device firmware.

> +	}
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930140833.576941-1-jtornosm@redhat.com?part=2