Re: [PATCH] hw/ppc: ppc440_uc: avoid multiply overflow in dcr_write_dma()

[BALATON Zoltan]

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. Maybe you could avoid it by 
changing the type of count or width to uint64_t instead of casting?

> 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 :-)

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);