On 01.08.2019 12:22, Chao Gao wrote:
> --- a/xen/arch/x86/microcode_intel.c
> +++ b/xen/arch/x86/microcode_intel.c
> @@ -134,14 +134,35 @@ static int collect_cpu_info(unsigned int cpu_num,
> struct cpu_signature *csig)
> return 0;
> }
>
> -static inline int microcode_update_match(
> - unsigned int cpu_num, const struct microcode_header_intel *mc_header,
> - int sig, int pf)
> +static enum microcode_match_result microcode_update_match(
> + const struct microcode_header_intel *mc_header, unsigned int sig,
> + unsigned int pf, unsigned int rev)
> {
> - struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu_num);
> -
> - return (sigmatch(sig, uci->cpu_sig.sig, pf, uci->cpu_sig.pf) &&
> - (mc_header->rev > uci->cpu_sig.rev));
> + const struct extended_sigtable *ext_header;
> + const struct extended_signature *ext_sig;
> + unsigned long data_size = get_datasize(mc_header);
> + unsigned int i;
> + const void *end = (const void *)mc_header + get_totalsize(mc_header);
> +
> + if ( sigmatch(sig, mc_header->sig, pf, mc_header->pf) )
> + return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;
Both here and ...
> + ext_header = (const void *)(mc_header + 1) + data_size;
> + ext_sig = (const void *)(ext_header + 1);
> +
> + /*
> + * Make sure there is enough space to hold an extended header and enough
> + * array elements.
> + */
> + if ( (end < (const void *)ext_sig) ||
> + (end < (const void *)(ext_sig + ext_header->count)) )
> + return MIS_UCODE;
> +
> + for ( i = 0; i < ext_header->count; i++ )
> + if ( sigmatch(sig, ext_sig[i].sig, pf, ext_sig[i].pf) )
> + return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;
... here there's again an assumption that there's strict ordering
between blobs, but as mentioned in reply to a later patch of an
earlier version this isn't the case. Therefore the function can't
be used to compare two arbitrary blobs, it may only be used to
compare a blob with what is already loaded into a CPU. I think it
is quite important to mention this restriction in a comment ahead
of the function.
The code itself looks fine to me, and a comment could perhaps be
added while committing; with such a comment
Reviewed-by: Jan Beulich <[email protected]>
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
