Re: [PATCH] xen/arm: Avoid integer overflow using MIDR_IMPLEMENTOR_MASK

[Julien Grall <julien@xxxxxxx>]

Hi Michal,

On 25/02/2022 08:38, Michal Orzel wrote:
> Value of macro MIDR_IMPLEMENTOR_MASK exceeds the range of integer
> and can lead to overflow. Currently there is no issue as it is used
> in an expression implicitly casted to u32 in MIDR_IS_CPU_MODEL_RANGE.
> To avoid possible problems, fix the macro.
Correct me if I am wrong, it sounds like this is only for hardening 
purpose at the moment.
As this code is coming from Linux, I would prefer if we first upstream 
to Linux and then port to Xen once merged.
> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> ---
>   xen/arch/arm/include/asm/processor.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/include/asm/processor.h 
> b/xen/arch/arm/include/asm/processor.h
> index 8ab2940f68..149fae0d27 100644
> --- a/xen/arch/arm/include/asm/processor.h
> +++ b/xen/arch/arm/include/asm/processor.h
> @@ -39,7 +39,7 @@
>   #define MIDR_VARIANT(midr) \
>       (((midr) & MIDR_VARIANT_MASK) >> MIDR_VARIANT_SHIFT)
>   #define MIDR_IMPLEMENTOR_SHIFT  24
> -#define MIDR_IMPLEMENTOR_MASK   (0xff << MIDR_IMPLEMENTOR_SHIFT)
> +#define MIDR_IMPLEMENTOR_MASK   (0xffu << MIDR_IMPLEMENTOR_SHIFT)
NIT: We tend to use 0xffU.

>   #define MIDR_IMPLEMENTOR(midr) \
>       (((midr) & MIDR_IMPLEMENTOR_MASK) >> MIDR_IMPLEMENTOR_SHIFT)
>   
Cheers,

--
Julien Grall