Re: [PATCH v17 09/12] vfio/pci: Hide TPH capability when TPH is unsupported

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Missing bounds check on dynamically-sized PCIe capabilities allows a malicious device to cause a kernel heap buffer overflow.
--

commit 2b14d080b02da7dcb99b160bf40eeb6f9e38b928
Author: Chengwen Feng <fengchengwen@huawei.com>

vfio/pci: Hide TPH capability when TPH is unsupported

This commit checks the device negotiated TPH support status before parsing
the TPH extended capability. It returns zero length to hide the capability
from userspace if TPH is disabled during topology negotiation.

> diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
> index a10ed733f0e3a..5c6ab172df6c0 100644
> --- a/drivers/vfio/pci/vfio_pci_config.c
> +++ b/drivers/vfio/pci/vfio_pci_config.c
[ ... ]
> @@ -1450,6 +1451,8 @@ static int vfio_ext_cap_len(struct vfio_pci_core_device *vdev, u16 ecap, u16 epo
>  		byte &= PCI_DPA_CAP_SUBSTATE_MASK;
>  		return PCI_DPA_BASE_SIZEOF + byte + 1;
>  	case PCI_EXT_CAP_ID_TPH:
> +		if (!pcie_tph_supported(pdev, false))
> +			return 0;
>  		ret = pci_read_config_dword(pdev, epos + PCI_TPH_CAP, &dword);

[Severity: Critical]
This is a pre-existing issue, but does this function safely bound the returned
capability length?

Looking at the rest of the PCI_EXT_CAP_ID_TPH case in vfio_ext_cap_len()
in drivers/vfio/pci/vfio_pci_config.c:

    if ((dword & PCI_TPH_CAP_LOC_MASK) == PCI_TPH_LOC_CAP) {
        int sts;

        sts = dword & PCI_TPH_CAP_ST_MASK;
        sts >>= PCI_TPH_CAP_ST_SHIFT;
        return PCI_TPH_BASE_SIZEOF + (sts * 2) + 2;
    }

Since the ST table size is read directly from the attacker-controlled device
config space, the returned length could be up to 4108 bytes.

Later, in vfio_ecap_init() in the same file, this unbounded length is used
to write into heap buffers:

    memset(map + epos, ecap, len);
    ret = vfio_fill_vconfig_bytes(vdev, epos, len);

Since the pci_config_map and vconfig arrays are allocated to pdev->cfg_size
(typically 4096 bytes), could a malicious PCIe endpoint or virtual device
provide an oversized TPH capability size to cause a kernel heap buffer
overflow, as there is no check ensuring epos + len fits within pdev->cfg_size?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616104621.41915-1-fengchengwen@huawei.com?part=9