Re: [PATCH] xen/ucode: Make Intel's microcode_sanity_check() stricter

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.09.2024 16:21, Andrew Cooper wrote:
> From: Demi Marie Obenour <demi@xxxxxxxxxxxxxxxxxxxxxx>
> 
> The SDM states that data size must be a multiple of 4, but Xen doesn't check
> this propery.
> 
> This is liable to cause a later failures, but should be checked explicitly.
> 
> Signed-off-by: Demi Marie Obenour <demi@xxxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

> --- a/xen/arch/x86/cpu/microcode/intel.c
> +++ b/xen/arch/x86/cpu/microcode/intel.c
> @@ -155,10 +155,13 @@ static int microcode_sanity_check(const struct 
> microcode_patch *patch)
>      uint32_t sum;
>  
>      /*
> -     * Total size must be a multiple of 1024 bytes.  Data size and the header
> -     * must fit within it.
> +     * The SDM states:
> +     * - Data size must be a multiple of 4.
> +     * - Total size must be a multiple of 1024 bytes.  Data size and the
> +     *   header must fit within it.
>       */
>      if ( (total_size & 1023) ||
> +         (data_size & 3) ||
>           data_size > (total_size - MC_HEADER_SIZE) )

And luckily get_totalsize() guarantees total_size > 0, for this
subtraction not to underflow. Maybe worth also mentioning in the
comment as you adjust it anyway.

Jan