On 01/29/21 20:28, Paolo Bonzini wrote:
> get_image_size() returns an int64_t, which pci_add_option_rom() assigns
> to an "int" without any range checking. A 32-bit BAR could be up to
> 2 GiB in size, so reject anything above it. In order to accomodate
> a rounded-up size of 2 GiB, change pci_patch_ids's size argument
> to unsigned.
>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> hw/pci/pci.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index a6b0c5602e..bbce10050b 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -25,6 +25,7 @@
> #include "qemu/osdep.h"
> #include "qemu-common.h"
> #include "qemu/datadir.h"
> +#include "qemu/units.h"
> #include "hw/irq.h"
> #include "hw/pci/pci.h"
> #include "hw/pci/pci_bridge.h"
> @@ -2258,7 +2258,7 @@ static uint8_t pci_find_capability_at_offset(PCIDevice
> *pdev, uint8_t offset)
>
> /* Patch the PCI vendor and device ids in a PCI rom image if necessary.
> This is needed for an option rom which is used for more than one device.
> */
> -static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, int size)
> +static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, uint32_t size)
> {
> uint16_t vendor_id;
> uint16_t device_id;
> @@ -2316,7 +2316,7 @@ static void pci_patch_ids(PCIDevice *pdev, uint8_t
> *ptr, int size)
> static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom,
> Error **errp)
> {
> - int size;
> + int64_t size;
> char *path;
> void *ptr;
> char name[32];
> @@ -2366,6 +2366,11 @@ static void pci_add_option_rom(PCIDevice *pdev, bool
> is_default_rom,
> error_setg(errp, "romfile \"%s\" is empty", pdev->romfile);
> g_free(path);
> return;
> + } else if (size > 2 * GiB) {
> + error_setg(errp, "romfile \"%s\" too large (size cannot exceed 2
> GiB)",
> + pdev->romfile);
> + g_free(path);
> + return;
> }
> size = pow2ceil(size);
>
>
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
