On Thu, 15 Feb 2024 at 18:04, Philippe Mathieu-Daudé <phi...@linaro.org> wrote:
>
> From: Clément Chigot <chi...@adacore.com>
>
> This adds a "ncpus" property to the "grlib-irqmp" device to be used
> later, this required a little refactoring of how we initialize the
> device (ie: use realize instead of init).
>
> Co-developed-by: Frederic Konrad <konrad.frede...@yahoo.fr>
> Signed-off-by: Clément Chigot <chi...@adacore.com>
> Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
> Message-ID: <20240131085047.18458-3-chi...@adacore.com>
> Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
Hi; Coverity points out a bug in this commit (CID 1534914):
> -static void grlib_irqmp_init(Object *obj)
> +static void grlib_irqmp_realize(DeviceState *dev, Error **errp)
> {
> - IRQMP *irqmp = GRLIB_IRQMP(obj);
> - SysBusDevice *dev = SYS_BUS_DEVICE(obj);
> + IRQMP *irqmp = GRLIB_IRQMP(dev);
>
> - qdev_init_gpio_in(DEVICE(obj), grlib_irqmp_set_irq, MAX_PILS);
> - qdev_init_gpio_out_named(DEVICE(obj), &irqmp->irq, "grlib-irq", 1);
> - memory_region_init_io(&irqmp->iomem, obj, &grlib_irqmp_ops, irqmp,
> + if ((!irqmp->ncpus) || (irqmp->ncpus > IRQMP_MAX_CPU)) {
> + error_setg(errp, "Invalid ncpus properties: "
> + "%u, must be 0 < ncpus =< %u.", irqmp->ncpus,
> + IRQMP_MAX_CPU);
> + }
We detect the out-of-range 'ncpus' value, but forget the "return"
statement, so execution will continue onward regardless, and
overrun the irqmp->irq[] array when we call qdev_init_gpio_out_named().
> +
> + qdev_init_gpio_in(dev, grlib_irqmp_set_irq, MAX_PILS);
> + qdev_init_gpio_out_named(dev, &irqmp->irq, "grlib-irq", 1);
> + memory_region_init_io(&irqmp->iomem, OBJECT(dev), &grlib_irqmp_ops,
> irqmp,
> "irqmp", IRQMP_REG_SIZE);
>
> irqmp->state = g_malloc0(sizeof *irqmp->state);
>
> - sysbus_init_mmio(dev, &irqmp->iomem);
> + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &irqmp->iomem);
> }
thanks
-- PMM
