Re: [PATCH for-5.1] hw/arm/armsse: Assert info->num_cpus is in-bounds in armsse_realize()

[Philippe Mathieu-Daudé]

On 7/13/20 4:37 PM, Peter Maydell wrote:
> In armsse_realize() we have a loop over [0, info->num_cpus), which
> indexes into various fixed-size arrays in the ARMSSE struct.  This
> confuses Coverity, which warns that we might overrun those arrays
> (CID 1430326, 1430337, 1430371, 1430414, 1430430).  This can't
> actually happen, because the info struct is always one of the entries
> in the armsse_variants[] array and num_cpus is either 1 or 2; we also
> already assert in armsse_init() that num_cpus is not too large.
> However, adding an assert to armsse_realize() like the one in
> armsse_init() should help Coverity figure out that these code paths
> aren't possible.

Similar to commit 1db889c71f ("hw/openrisc/openrisc_sim: Add assertion
to silence GCC warning").

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/armsse.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
> index 64fcab895f7..dcbff9bd8f4 100644
> --- a/hw/arm/armsse.c
> +++ b/hw/arm/armsse.c
> @@ -452,6 +452,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>  
> +    assert(info->num_cpus <= SSE_MAX_CPUS);
> +
>      /* max SRAM_ADDR_WIDTH: 24 - log2(SRAM_NUM_BANK) */
>      assert(is_power_of_2(info->sram_banks));
>      addr_width_max = 24 - ctz32(info->sram_banks);
>