On 8/5/22 21:30, BALATON Zoltan wrote:
On Fri, 5 Aug 2022, Daniel Henrique Barboza wrote:
Coverity reports a OVERFLOW_BEFORE_WIDEN issue in dcr_write_dma(). When
handling the DMA0_CR switch we're doing a multiplication between two
integers (count and width), and the product is assigned to an uint64_t
(xferlen). The int32 product can be overflow before widened.
It can't in practice though as count is max 0xffff and width is max 8 so this sounds like a Coverity false positive.
This time I believe it's worth fixing it. This code can be used as a base
to do something similar somewhere else, where the variables don't have
hard caps like we have here, and then we'll have a real overflow to
deal with.
Maybe you could avoid it by changing the type of count or width to uint64_t
instead of casting?
That was my first idea but it would require more changes. I'll send a
v2.
Fix it by casting the first operand to uint64_t to force the product to
be 64 bit.
Fixes: Coverity CID 1490852
Fixes: 3c409c1927ef ("ppc440_uc: Basic emulation of PPC440 DMA controller")
This line does not appear in 3c409c1927ef so this second Fixes line is bogus.
This was added to fix other Coverity issues in eda3f17bcd7b96 but still did not
make Coverity happy :-)
My bad. I'll drop that "Fixes" line.
Daniel
Regards,
BALATON Zoltan
Cc: BALATON Zoltan <balaton@eik.bme.hu>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
hw/ppc/ppc440_uc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
index 11fdb88c22..31eeffa946 100644
--- a/hw/ppc/ppc440_uc.c
+++ b/hw/ppc/ppc440_uc.c
@@ -908,7 +908,7 @@ static void dcr_write_dma(void *opaque, int dcrn, uint32_t
val)
sidx = didx = 0;
width = 1 << ((val & DMA0_CR_PW) >> 25);
- xferlen = count * width;
+ xferlen = (uint64_t)count * width;
wlen = rlen = xferlen;
rptr = cpu_physical_memory_map(dma->ch[chnl].sa, &rlen,
false);