Re: [PATCH 1/3] hw/m68k/mcf5208: Avoid shifting off end of integer

[Thomas Huth]

Am Fri, 30 Aug 2024 18:34:50 +0100
schrieb Peter Maydell <peter.maydell@linaro.org>:

> In m5208_sys_read(), we have a loop of n from 0 to 31, and we
> calculate (2u << n).  For the n == 31 iteration this will shift off
> the top of the unsigned 32 bit integer.
> 
> This is harmless, because we're going to stop the loop with n == 31
> anyway, but we can avoid the error by using 64-bit arithmetic here.
> 
> (The SDCS0 register is documented at
> https://www.nxp.com/docs/en/reference-manual/MCF5208RM.pdf
> section 18.4.5; we want the lower 5 bits to indicate the
> RAM size, where 31 == 4GB, 30 == 2GB, and so on down.
> As it happens, the layout of the mcf5208evb board memory map
> means it doesn't make sense to have more than 1GB of RAM
> in any case.)
> 
> Resolves: Coverity CID 1547727
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/m68k/mcf5208.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
> index ec14096aa43..0ad347dfa81 100644
> --- a/hw/m68k/mcf5208.c
> +++ b/hw/m68k/mcf5208.c
> @@ -158,7 +158,7 @@ static uint64_t m5208_sys_read(void *opaque, hwaddr addr,
>          {
>              int n;
>              for (n = 0; n < 32; n++) {
> -                if (current_machine->ram_size < (2u << n)) {
> +                if (current_machine->ram_size < (2ULL << n)) {
>                      break;
>                  }
>              }

Reviewed-by: Thomas Huth <huth@tuxfamily.org>