Re: [PATCH] target/arm/tcg: fix potential integer overflow in iwmmxt_macuw()

[Richard Henderson]

On 12/12/24 08:27, gerben@altlinux.org wrote:
> From: Denis Rastyogin <gerben@altlinux.org>
>
> The function iwmmxt_macuw() could potentially cause an integer
> overflow when summing up four 32-bit multiplications.
> This occurs because the intermediate results may exceed the 32-bit
> range before being cast to uint64_t. The fix ensures each
> multiplication is explicitly cast to uint64_t prior to summation,
> preventing potential issues and ensuring correctness.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Signed-off-by: Denis Sergeev <zeff@altlinux.org>
> Signed-off-by: Denis Rastyogin <gerben@altlinux.org>
> ---
>   target/arm/tcg/iwmmxt_helper.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/arm/tcg/iwmmxt_helper.c b/target/arm/tcg/iwmmxt_helper.c
> index 610b1b2103..19c709655e 100644
> --- a/target/arm/tcg/iwmmxt_helper.c
> +++ b/target/arm/tcg/iwmmxt_helper.c
> @@ -140,7 +140,7 @@ uint64_t HELPER(iwmmxt_macsw)(uint64_t a, uint64_t b)
>   
>   uint64_t HELPER(iwmmxt_macuw)(uint64_t a, uint64_t b)
>   {
> -#define MACU(SHR) ( \
> +#define MACU(SHR) (uint64_t)( \
>           (uint32_t) ((a >> SHR) & 0xffff) * \
>           (uint32_t) ((b >> SHR) & 0xffff))
>       return MACU(0) + MACU(16) + MACU(32) + MACU(48);

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~