Re: [PATCH 04/15] PCI: mvebu: Handle invalid size of read config request

Bjorn Helgaas
2022-01-07T18:45:55Z

On Thu, Nov 25, 2021 at 01:45:54PM +0100, Pali Rohár wrote:
> Function mvebu_pcie_hw_rd_conf() does not handle invalid size. So correctly
> set read value to all-ones and return appropriate error return value
> PCIBIOS_BAD_REGISTER_NUMBER like in mvebu_pcie_hw_wr_conf() function.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Cc: stable@vger.kernel.org

Is there a bug that this fixes? If not, I would drop the stable tag
(as I see Lorenzo already did, thanks!).

> ---
> drivers/pci/controller/pci-mvebu.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
> index 08274132cdfb..19c6ee298442 100644
> --- a/drivers/pci/controller/pci-mvebu.c
> +++ b/drivers/pci/controller/pci-mvebu.c
> @@ -261,6 +261,9 @@ static int mvebu_pcie_hw_rd_conf(struct mvebu_pcie_port *port,
> case 4:
> *val = readl_relaxed(conf_data);
> break;
> + default:
> + *val = 0xffffffff;
> + return PCIBIOS_BAD_REGISTER_NUMBER;

Might be the right thing to do, but there are many config accessors
that do not set *val to ~0 before returning
PCIBIOS_BAD_REGISTER_NUMBER:

pci_bus_read_config_byte (and word, dword) # PCI_OP_READ(), *val unchanged
pci_generic_config_read # *val = 32-bit value
pci_user_read_config_byte (...) # PCI_USER_READ_CONFIG(), *val unchanged
sh7786_pcie_read # *val unchanged
dw_pcie_read # *val = 0
mobiveil_pcie_read # *val = 0
faraday_raw_pci_read_config # *val = 32-bit value
ixp4xx_pci_read_config # *val unchanged
orion5x_pci_hw_rd_conf # *val = 32-bit value
orion_pcie_rd_conf # *val = 32-bit value
bonito64_pcibios_read # *val = 32-bit value
loongson_pcibios_read # *val = 32-bit value
msc_pcibios_read # *val = 32-bit value
ar724x_pci_read # *val unchanged
bcm1480_pcibios_read # *val = 32-bit value
_altera_pcie_cfg_read # *val = 32-bit value
rockchip_pcie_rd_own_conf # *val = 0
rockchip_pcie_rd_other_conf # *val = 0
pci_bridge_emul_conf_read # may depend on op?

There are more, but I got tired of looking. I actually didn't see any
that set *val to ~0.

I think the check in PCI_OP_READ() means that most accessors will
never see an invalid "size".

> }
>
> return PCIBIOS_SUCCESSFUL;
> --
> 2.20.1
>