Re: [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero

[Cédric Le Goater]

On 6/25/24 8:00 AM, cmd wrote:
> Hi
>
> On 25/06/2024 03:50, Jamin Lin via wrote:
> > Coverity reports a possible DIVIDE_BY_ZERO issue regarding the
> > "ram_size" object property. This can not happen because RAM has
> > predefined valid sizes per SoC. Nevertheless, add a test to
> > close the issue.
> >
> > Fixes: Coverity CID 1547113
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > Reviewed-by: Cédric Le Goater <clg@redhat.com>
> > [ clg: Rewrote commit log ]
> > Signed-off-by: Cédric Le Goater <clg@redhat.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;
> > +    }
> > +
> If we are sure that the error cannot happen, shouldn't we assert instead?

Yes. That is what Peter suggested. This needs to be changed.


Thanks,

C.



> >       /*
> >        * Emulate ddr capacity hardware behavior.
> >        * If writes the data to the address which is beyond the ram size,