On 6/25/24 8:15 AM, Jamin Lin wrote:
Hi cmd, Cedric and Peter,
-----Original Message-----
From: cmd <clement.mathieudrif....@gmail.com>
Sent: Tuesday, June 25, 2024 2:07 PM
To: Cédric Le Goater <c...@kaod.org>; Jamin Lin <jamin_...@aspeedtech.com>;
Peter Maydell <peter.mayd...@linaro.org>; Steven Lee
<steven_...@aspeedtech.com>; Troy Lee <leet...@gmail.com>; Andrew
Jeffery <and...@codeconstruct.com.au>; Joel Stanley <j...@jms.id.au>; open
list:ASPEED BMCs <qemu-...@nongnu.org>; open list:All patches CC here
<qemu-devel@nongnu.org>
Cc: Cédric Le Goater <c...@redhat.com>
Subject: Re: [PATCH v2 1/2] aspeed/soc: Fix possible divide by zero
On 25/06/2024 08:03, Cédric Le Goater wrote:
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_...@aspeedtech.com>
Reviewed-by: Cédric Le Goater <c...@redhat.com> [ clg: Rewrote commit
log ]
Signed-off-by: Cédric Le Goater <c...@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_a
bort);
+ 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 for review and suggestion.
How about this change?
assert(ram_size > 0);
yes.
I will send another patch fixing a long standing issue in the SDMC
model not checking the ram_size value in the realize handler. It
relies on the "ram-size" property being set.
Thanks,
C.
If you agree, I will send v3 patch.
Thanks-Jamin
Thanks,
C.
Ok fine, I didn't see the message, sorry!
Thanks
>cmd
/*
* Emulate ddr capacity hardware behavior.
* If writes the data to the address which is beyond the ram
size,
