Re: [PATCH RESEND v18 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:
- [High] Pre-existing kernel heap buffer overflow in `vfio_ecap_init()` due to unvalidated extended capability lengths.
--

commit 04070fd3c8083d2b670da1000c99d6fc517e4114
Author: Chengwen Feng <fengchengwen@huawei.com>

    vfio/pci: Hide TPH capability when TPH is unsupported
    
    Check the device negotiated TPH support status before parsing the TPH
    extended capability. Return 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
> @@ -22,6 +22,7 @@
>  
>  #include <linux/fs.h>
>  #include <linux/pci.h>
> +#include <linux/pci-tph.h>
>  #include <linux/uaccess.h>
>  #include <linux/vfio.h>
>  #include <linux/slab.h>

[ ... ]

> @@ -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: High]
This isn't a bug introduced by this patch, but I noticed a pre-existing
issue in how the returned capability length is handled.

Does the returned length from vfio_ext_cap_len() need to be validated
against pdev->cfg_size?

In vfio_ecap_init(), the length returned here is used directly:

drivers/vfio/pci/vfio_pci_config.c:vfio_ecap_init() {
    ...
		len = vfio_ext_cap_len(vdev, ecap, epos);
		if (!len) {
			...
			continue;
		}
		...
		memset(map + epos, ecap, len);
		ret = vfio_fill_vconfig_bytes(vdev, epos, len);
    ...
}

If a malicious or buggy PCIe device advertises an extended capability like
TPH with a large ST table length (e.g., up to 4108 bytes), could epos + len
exceed pdev->cfg_size?

If so, it appears the subsequent memset() and vfio_fill_vconfig_bytes()
operations could write out of bounds into the map and vconfig allocations.

>  		if (ret)
>  			return pcibios_err_to_errno(ret);

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