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

[Aravind Gopalakrishnan <aravind.gopalakrishnan@xxxxxxx>]

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.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel