Re: [Xen-devel] [PATCH v10 01/16] microcode/intel: extend microcode_update_match()
On Fri, Sep 13, 2019 at 08:50:59AM +0200, Jan Beulich wrote:
>On 12.09.2019 12:24, Jan Beulich wrote:
>> On 12.09.2019 09:22, Chao Gao wrote:
>>> --- a/xen/arch/x86/microcode_intel.c
>>> +++ b/xen/arch/x86/microcode_intel.c
>>> @@ -134,21 +134,11 @@ 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 int microcode_sanity_check(const void *mc)
>>> {
>>> -struct ucode_cpu_info *uci = _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));
>>> -}
>>> -
>>> -static int microcode_sanity_check(void *mc)
>>> -{
>>> -struct microcode_header_intel *mc_header = mc;
>>> -struct extended_sigtable *ext_header = NULL;
>>> -struct extended_signature *ext_sig;
>>> +const struct microcode_header_intel *mc_header = mc;
>>> +const struct extended_sigtable *ext_header = NULL;
>>> +const struct extended_signature *ext_sig;
>>> unsigned long total_size, data_size, ext_table_size;
>>> unsigned int ext_sigcount = 0, i;
>>> uint32_t sum, orig_sum;
>>> @@ -234,6 +224,42 @@ static int microcode_sanity_check(void *mc)
>>> return 0;
>>> }
>>>
>>> +/* Check an update against the CPU signature and current update revision */
>>> +static enum microcode_match_result microcode_update_match(
>>> +const struct microcode_header_intel *mc_header, unsigned int cpu)
>>> +{
>>> +const struct extended_sigtable *ext_header;
>>> +const struct extended_signature *ext_sig;
>>> +unsigned int i;
>>> +struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu);
>>> +unsigned int sig = uci->cpu_sig.sig;
>>> +unsigned int pf = uci->cpu_sig.pf;
>>> +unsigned int rev = uci->cpu_sig.rev;
>>> +unsigned long data_size = get_datasize(mc_header);
>>> +const void *end = (const void *)mc_header + get_totalsize(mc_header);
>>> +
>>> +ASSERT(!microcode_sanity_check(mc_header));
>>> +if ( sigmatch(sig, mc_header->sig, pf, mc_header->pf) )
>>> +return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;
>>> +
>>> +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;
>>
>> With you now assuming that the blob has previously passed
>> microcode_sanity_check(), this only needs to be
>>
>> if ( (end <= (const void *)ext_sig) )
>> return MIS_UCODE;
>>
>> now afaict.
>>
>> Reviewed-by: Jan Beulich
>> preferably with this adjustment (assuming you agree).
>
>FAOD: I'd be happy to make the adjustment while committing, but
>I'd like to have your consent (or you proving me wrong). This
>would, as it looks, allow everything up to patch 8 to go in.
Please go ahead. Thanks
Chao
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v10 01/16] microcode/intel: extend microcode_update_match()
On 12.09.2019 12:24, Jan Beulich wrote:
> On 12.09.2019 09:22, Chao Gao wrote:
>> --- a/xen/arch/x86/microcode_intel.c
>> +++ b/xen/arch/x86/microcode_intel.c
>> @@ -134,21 +134,11 @@ 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 int microcode_sanity_check(const void *mc)
>> {
>> -struct ucode_cpu_info *uci = _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));
>> -}
>> -
>> -static int microcode_sanity_check(void *mc)
>> -{
>> -struct microcode_header_intel *mc_header = mc;
>> -struct extended_sigtable *ext_header = NULL;
>> -struct extended_signature *ext_sig;
>> +const struct microcode_header_intel *mc_header = mc;
>> +const struct extended_sigtable *ext_header = NULL;
>> +const struct extended_signature *ext_sig;
>> unsigned long total_size, data_size, ext_table_size;
>> unsigned int ext_sigcount = 0, i;
>> uint32_t sum, orig_sum;
>> @@ -234,6 +224,42 @@ static int microcode_sanity_check(void *mc)
>> return 0;
>> }
>>
>> +/* Check an update against the CPU signature and current update revision */
>> +static enum microcode_match_result microcode_update_match(
>> +const struct microcode_header_intel *mc_header, unsigned int cpu)
>> +{
>> +const struct extended_sigtable *ext_header;
>> +const struct extended_signature *ext_sig;
>> +unsigned int i;
>> +struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu);
>> +unsigned int sig = uci->cpu_sig.sig;
>> +unsigned int pf = uci->cpu_sig.pf;
>> +unsigned int rev = uci->cpu_sig.rev;
>> +unsigned long data_size = get_datasize(mc_header);
>> +const void *end = (const void *)mc_header + get_totalsize(mc_header);
>> +
>> +ASSERT(!microcode_sanity_check(mc_header));
>> +if ( sigmatch(sig, mc_header->sig, pf, mc_header->pf) )
>> +return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;
>> +
>> +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;
>
> With you now assuming that the blob has previously passed
> microcode_sanity_check(), this only needs to be
>
> if ( (end <= (const void *)ext_sig) )
> return MIS_UCODE;
>
> now afaict.
>
> Reviewed-by: Jan Beulich
> preferably with this adjustment (assuming you agree).
FAOD: I'd be happy to make the adjustment while committing, but
I'd like to have your consent (or you proving me wrong). This
would, as it looks, allow everything up to patch 8 to go in.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v10 01/16] microcode/intel: extend microcode_update_match()
On 12.09.2019 09:22, Chao Gao wrote:
> --- a/xen/arch/x86/microcode_intel.c
> +++ b/xen/arch/x86/microcode_intel.c
> @@ -134,21 +134,11 @@ 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 int microcode_sanity_check(const void *mc)
> {
> -struct ucode_cpu_info *uci = _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));
> -}
> -
> -static int microcode_sanity_check(void *mc)
> -{
> -struct microcode_header_intel *mc_header = mc;
> -struct extended_sigtable *ext_header = NULL;
> -struct extended_signature *ext_sig;
> +const struct microcode_header_intel *mc_header = mc;
> +const struct extended_sigtable *ext_header = NULL;
> +const struct extended_signature *ext_sig;
> unsigned long total_size, data_size, ext_table_size;
> unsigned int ext_sigcount = 0, i;
> uint32_t sum, orig_sum;
> @@ -234,6 +224,42 @@ static int microcode_sanity_check(void *mc)
> return 0;
> }
>
> +/* Check an update against the CPU signature and current update revision */
> +static enum microcode_match_result microcode_update_match(
> +const struct microcode_header_intel *mc_header, unsigned int cpu)
> +{
> +const struct extended_sigtable *ext_header;
> +const struct extended_signature *ext_sig;
> +unsigned int i;
> +struct ucode_cpu_info *uci = _cpu(ucode_cpu_info, cpu);
> +unsigned int sig = uci->cpu_sig.sig;
> +unsigned int pf = uci->cpu_sig.pf;
> +unsigned int rev = uci->cpu_sig.rev;
> +unsigned long data_size = get_datasize(mc_header);
> +const void *end = (const void *)mc_header + get_totalsize(mc_header);
> +
> +ASSERT(!microcode_sanity_check(mc_header));
> +if ( sigmatch(sig, mc_header->sig, pf, mc_header->pf) )
> +return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;
> +
> +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;
With you now assuming that the blob has previously passed
microcode_sanity_check(), this only needs to be
if ( (end <= (const void *)ext_sig) )
return MIS_UCODE;
now afaict.
Reviewed-by: Jan Beulich
preferably with this adjustment (assuming you agree).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
