Re: [PATCH v11 2/5] PCI/TPH: Export pcie_tph_get_st_modes() for external use

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unchecked return value of pci_read_config_dword() leads to reporting false capabilities and bypassing safety checks on disconnected devices.
--

commit 7d284f4750da4fed3caa1876a0ac6b5f46c96ad9
Author: Chengwen Feng <fengchengwen@huawei.com>

PCI/TPH: Export pcie_tph_get_st_modes() for external use

Export the helper to retrieve supported PCIe TPH steering tag modes so that
drivers like VFIO can query and expose device capabilities to userspace.
Add stub functions for when CONFIG_PCIE_TPH is disabled, and add validation
to prevent invalid configuration space access.

> diff --git a/drivers/pci/tph.c b/drivers/pci/tph.c
> index 877cf556242b4..ba31b010f67ad 100644
> --- a/drivers/pci/tph.c
> +++ b/drivers/pci/tph.c
[ ... ]
> @@ -145,15 +145,27 @@ static void set_ctrl_reg_req_en(struct pci_dev *pdev, u8 req_type)
[ ... ]
> +u8 pcie_tph_get_st_modes(struct pci_dev *pdev)
>  {
>  	u32 reg;
>  
> +	if (!pdev->tph_cap)
> +		return 0;
> +
>  	pci_read_config_dword(pdev, pdev->tph_cap + PCI_TPH_CAP, &reg);
>  	reg &= PCI_TPH_CAP_ST_NS | PCI_TPH_CAP_ST_IV | PCI_TPH_CAP_ST_DS;
>  
>  	return reg;
>  }
> +EXPORT_SYMBOL(pcie_tph_get_st_modes);

Is it possible for pci_read_config_dword() to fail here?

If the device is disconnected or the read fails, reg could be set to
0xFFFFFFFF (PCI_ERROR_RESPONSE). It might also be left uninitialized if the
host controller fails the read without updating the variable.

Because the return value of pci_read_config_dword() is not checked, a value
of 0xFFFFFFFF in reg would cause this function to return 0x7 after masking.

With this function now being exported to external drivers like VFIO, could
this result in falsely exposing all ST modes as supported to userspace
instead of returning 0 on failure?

> @@ -168,6 +180,9 @@ u32 pcie_tph_get_st_table_loc(struct pci_dev *pdev)
>  {
>  	u32 reg;
>  
> +	if (!pdev->tph_cap)
> +		return PCI_TPH_LOC_NONE;
> +
>  	pci_read_config_dword(pdev, pdev->tph_cap + PCI_TPH_CAP, &reg);
>  
>  	return reg & PCI_TPH_CAP_LOC_MASK;
>  }

Similarly, if pci_read_config_dword() fails here and sets reg to 0xFFFFFFFF,
this will return PCI_TPH_CAP_LOC_MASK (a reserved location) rather than
PCI_TPH_LOC_NONE.

Could this incorrect return value bypass safety checks that expect
PCI_TPH_LOC_NONE for invalid devices, or cause pcie_enable_tph() to
improperly enable TPH when hardware access has failed?

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