Re: [PULL 34/56] hw/intc/grlib_irqmp: add ncpus property

[Peter Maydell]

On Thu, 15 Feb 2024 at 18:04, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> From: Clément Chigot <chigot@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.frederic@yahoo.fr>
> Signed-off-by: Clément Chigot <chigot@adacore.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Message-ID: <20240131085047.18458-3-chigot@adacore.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@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