Re: [PATCH for-4.15] x86/ucode/amd: Fix OoB read in cpu_request_microcode()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 10.02.2021 00:40, Andrew Cooper wrote:
> verify_patch_size() is a maximum size check, and doesn't have a minimum bound.
> 
> If the microcode container encodes a blob with a length less than 64 bytes,
> the subsequent calls to microcode_fits()/compare_header() may read off the end
> of the buffer.
> 
> Fixes: 4de936a38a ("x86/ucode/amd: Rework parsing logic in 
> cpu_request_microcode()")
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

> --- a/xen/arch/x86/cpu/microcode/amd.c
> +++ b/xen/arch/x86/cpu/microcode/amd.c
> @@ -349,6 +349,7 @@ static struct microcode_patch 
> *cpu_request_microcode(const void *buf, size_t siz
>              if ( size < sizeof(*mc) ||
>                   (mc = buf)->type != UCODE_UCODE_TYPE ||
>                   size - sizeof(*mc) < mc->len ||
> +                 mc->len < sizeof(struct microcode_patch) ||

I was inclined to suggest to use <= here, but I guess a blob
with 1 byte of data is as bogus as one with 0 bytes of data.

Jan