Re: [PATCH v1 1/2] aspeed/soc: fix coverity issue

[Cédric Le Goater]

On 6/24/24 4:01 PM, Peter Maydell wrote:
> On Mon, 24 Jun 2024 at 14:58, Cédric Le Goater <clg@kaod.org> wrote:
> > On 6/24/24 2:18 PM, Peter Maydell wrote:
> > > On Wed, 19 Jun 2024 at 10:35, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> > > > Fix coverity defect: DIVIDE_BY_ZERO.
> > > >
> > > > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > > > ---
> > > >    hw/arm/aspeed_ast27x0.c | 6 ++++++
> > > >    1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
> > > > index b6876b4862..d14a46df6f 100644
> > > > --- a/hw/arm/aspeed_ast27x0.c
> > > > +++ b/hw/arm/aspeed_ast27x0.c
> > > > @@ -211,6 +211,12 @@ static void aspeed_ram_capacity_write(void *opaque, hwaddr 
> > > > addr, uint64_t data,
> > > >        ram_size = object_property_get_uint(OBJECT(&s->sdmc), "ram-size",
> > > >                                            &error_abort);
> > > >
> > > > +    if (!ram_size) {
> > > > +        qemu_log_mask(LOG_GUEST_ERROR,
> > > > +                      "%s: ram_size is zero",  __func__);
> > > > +        return;
> > > > +    }
> > > > +
> > >
> > > Isn't this a QEMU bug rather than a guest error? The
> > > RAM size presumably should never be zero unless the board
> > > set the ram-size property on the SDMC incorrectly. So the
> > > SDMC device should check (and return an error from its realize
> > > method) that the ram-size property is valid,
> >
> > That's the case in aspeed_sdmc_set_ram_size() which is called from
> > the aspeed machine init routine when the ram size is set.
>
> True, but if the property is never set at all then the
> struct field will be left at whatever value it had, which
> is 0, I think. So if that's not valid then it either needs
> to be a different default or else the realize method should
> complain that the property was never set.

Ah, yes, and the issue has been there for while. I will send a separate
patch for this.

Thanks,

C.