Re: [PATCH V2] x86, amd_ucode: Support multiple container files appended together

[Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>]

On 6/24/2014 11:04 PM, Boris Ostrovsky wrote:
>
>>>>   -    /* No more data */
>>>> -    if ( mpbuf->len + 12 >= *offset )
>>>> -        return -EINVAL;
>>> Iirc you and Boris agreed that this check is pointless _here_. But I
>>> doubt it can be removed without replacement elsewhere.
>>
>> For single containers, this check made some sense earlier as we 
>> verify to see there is *some*
>> data beyond the equivalent_table structure.
>> Say, mpbuf->len=0 and we return error val; Due to the fact that we 
>> have already advanced *offset,
>> cases when we reach EOF or *offset goes over bufsize is handled in 
>> container_fast_forward
>> function.
>>
>> For multiple containers, we will always have at least two such 
>> container headers and hence,
>> mpbuf->len + 12 is always less than total_size
>>
>> If first container for some reason is corrupted and exposes 
>> mpbuf->len=0,  we return EINVAL
>> and forward to next container.
>> (This is infact one reason to  advance *offset earlier. See below)
>>
>> Now, if the last container were to have mpbuf->len=0,
>> As Boris mentioned on earlier thread, we will
>> continue because 'if (0+12 >= tot_size) ' is false.
>> Here too, we will return EINVAL.
>>
>> Again, advancing *offset early allows to workaround these issues.
>> And this check can be removed as a result.
>
> Let's say we have a single container and the file got truncated (i.e. 
> bufsize in cpu_request_microcode() is smaller than it should be). 
> Aren't we now risking doing a memcpy out of too short a buffer?
>
>

No, because in 'install_equiv_cpu_table', we only alloc memory and 
memcpy data for the equiv_cpu_table.
Alloc-ing memory (and memcpy) for the patch is handled by 
get_ucode_from_buffer_amd;
and corrupted files (like the ones you say) should be handled by this-
  if ( (off + mpbuf->len) > bufsize )
{
    printk(KERN_ERR "microcode: Bad data in microcode data file\n");
    return -EINVAL;
}

-Aravind.