[PATCH] pci: Add checks to prevent config space overflow
Stefan Roese
sr at denx.de
Mon Jul 4 08:00:23 CEST 2022
On 03.07.22 12:48, Pali Rohár wrote:
> PCIe config space has address range 0-4095. So do not allow reading from
> addresses outside of this range. Lot of U-Boot drivers do not expect that
> passed value is not in this range. PCI DM read function is exetended to
s/exetended/extended
> fill read value to all ones or zeros when it fails as U-Boot callers
> ignores return value.
>
> Calling U-Boot command 'pci display.b 0.0.0 0 0x2000' now stops printing
> config space at the end (before 0x1000 address).
>
> Signed-off-by: Pali Rohár <pali at kernel.org>
> ---
> cmd/pci.c | 16 ++++++++++++++--
> drivers/pci/pci-uclass.c | 10 +++++++++-
> 2 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/cmd/pci.c b/cmd/pci.c
> index a99e8f8ad6e0..6258699fec81 100644
> --- a/cmd/pci.c
> +++ b/cmd/pci.c
> @@ -358,6 +358,9 @@ static int pci_cfg_display(struct udevice *dev, ulong addr,
> if (length == 0)
> length = 0x40 / byte_size; /* Standard PCI config space */
>
> + if (addr >= 4096)
> + return 1;
> +
> /* Print the lines.
> * once, and all accesses are with the specified bus width.
> */
> @@ -378,7 +381,10 @@ static int pci_cfg_display(struct udevice *dev, ulong addr,
> rc = 1;
> break;
> }
> - } while (nbytes > 0);
> + } while (nbytes > 0 && addr < 4096);
> +
> + if (rc == 0 && nbytes > 0)
> + return 1;
>
> return (rc);
> }
> @@ -390,6 +396,9 @@ static int pci_cfg_modify(struct udevice *dev, ulong addr, ulong size,
> int nbytes;
> ulong val;
>
> + if (addr >= 4096)
> + return 1;
> +
> /* Print the address, followed by value. Then accept input for
> * the next value. A non-converted value exits.
> */
> @@ -427,7 +436,10 @@ static int pci_cfg_modify(struct udevice *dev, ulong addr, ulong size,
> addr += size;
> }
> }
> - } while (nbytes);
> + } while (nbytes && addr < 4096);
> +
> + if (nbytes)
> + return 1;
>
> return 0;
> }
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index 89245a271e16..7402079471c8 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -288,6 +288,8 @@ int pci_bus_write_config(struct udevice *bus, pci_dev_t bdf, int offset,
> ops = pci_get_ops(bus);
> if (!ops->write_config)
> return -ENOSYS;
> + if (offset < 0 || offset >= 4096)
> + return -EINVAL;
> return ops->write_config(bus, bdf, offset, value, size);
> }
>
> @@ -366,8 +368,14 @@ int pci_bus_read_config(const struct udevice *bus, pci_dev_t bdf, int offset,
> struct dm_pci_ops *ops;
>
> ops = pci_get_ops(bus);
> - if (!ops->read_config)
> + if (!ops->read_config) {
> + *valuep = pci_conv_32_to_size(~0, offset, size);
> return -ENOSYS;
> + }
> + if (offset < 0 || offset >= 4096) {
> + *valuep = pci_conv_32_to_size(0, offset, size);
> + return -EINVAL;
> + }
How about introducing a macro for this 4096 max value instead?
Other than this:
Reviewed-by: Stefan Roese <sr at denx.de>
Thanks,
Stefan
More information about the U-Boot
mailing list