Re: [Xen-devel] [PATCH v10 01/16] microcode/intel: extend microcode_update_match()

[Chao Gao <chao.gao@xxxxxxxxx>]

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 = &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));
>>> -}
>>> -
>>> -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 = &per_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 <jbeulich@xxxxxxxx>
>> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel