Re: [PATCH v8 3/7] PCI/TPH: Fix pcie_tph_get_st_table_size() for MSI-X table location

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Modifying pcie_tph_get_st_table_size() to return > 0 for MSI-X located ST tables causes severe out-of-bounds read/writes in PCI configuration space during TPH state save and restore.
--

commit 73d4864dbc8b240284d0329056ab114562107193
Author: Chengwen Feng <fengchengwen@huawei.com>

    PCI/TPH: Fix pcie_tph_get_st_table_size() for MSI-X table location
    
    pcie_tph_get_st_table_size() previously only returned valid size when ST
    Table is in TPH Capability space. According to PCIe spec [1], ST table size
    is also valid when ST Table is located in MSI-X table.

[ ... ]

> diff --git a/drivers/pci/tph.c b/drivers/pci/tph.c
> index ba31b010f67ad..de5bd7039cdca 100644
> --- a/drivers/pci/tph.c
> +++ b/drivers/pci/tph.c

[ ... ]

> @@ -200,7 +201,7 @@ u16 pcie_tph_get_st_table_size(struct pci_dev *pdev)
>  
>  	/* Check ST table location first */
>  	loc = pcie_tph_get_st_table_loc(pdev);
> -	if (loc != PCI_TPH_LOC_CAP)
> +	if (loc != PCI_TPH_LOC_CAP && loc != PCI_TPH_LOC_MSIX)
>  		return 0;
>  
>  	pci_read_config_dword(pdev, pdev->tph_cap + PCI_TPH_CAP, &reg);

If pcie_tph_get_st_table_size() returns a value greater than 0 when the ST
table is located in the MSI-X table, does this cause pci_restore_tph_state()
to write out of bounds in the PCI configuration space?

Looking at pci_restore_tph_state(), it uses the size returned by
pcie_tph_get_st_table_size() to write ST entries directly into the TPH
Extended Capability:

drivers/pci/tph.c:pci_restore_tph_state() {
	...
	st_entry = (u16 *)cap;
	offset = PCI_TPH_BASE_SIZEOF;
	num_entries = pcie_tph_get_st_table_size(pdev);
	for (i = 0; i < num_entries; i++) {
		pci_write_config_word(pdev, pdev->tph_cap + offset,
				      *st_entry++);
		offset += sizeof(u16);
	}
}

When the ST table is located in MSI-X, the entries are memory mapped rather
than stored in the TPH capability itself.

Because the TPH Extended Capability is only 12 bytes long in this case, could
this loop clobber subsequent capabilities in the configuration space during
resume?

A similar out-of-bounds read would occur in pci_save_tph_state().

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