[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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




+    while ( (error = install_equiv_cpu_table(mc_amd, buf, &tot_size,
+                                             &offset)) == 0 )

install_equiv_cpu_table() checks whether container size is larger than the buffer size.
Not really.
It is basically checking if we have data at all to parse (like the comment says:/* No more data */)

If we have multiple containers merged I think tot_size is the size of the merged file, not of an individual container.

That's right.

And this makes the first check in install_equiv_cpu_table() not especially meaningful.


Theoretically-
If it so happens that we don't have any patch files and just the container header,
then mpbuf->len would be zero and
 if ( mpbuf->len + CONT_HDR_SIZE >= *tot_size )
will fail in this case- (which gives some meaning to the check being there) 1. If containers are concatenated, but there is no matching patch on the first one
2. So, we fast-fwd to next container file but this one has mpbuf->len=0

So if you have two merged containers, the first one being 1K and the second one empty, then while processing the second container you will instead of returning -EINVAL continue because 'if (0+12 >= 1K) ' will evaluate to false. And it seems to me that you then may well go beyond the end of the buffer.



Which leads me to another scenario though-
1. If we do have multiple containers and if right patch is not on the first one, but on some subsequent container
2. If the first container has mpbuf->len=0
Then we'd just error out right there. Hmm.. I'll have to re-work the logic then.

Isn't it the other way around? We won't error out, we will continue. Which is wrong.


Come to think, if the first container is corrupted for some reason and returns error (or there's an -ENOMEM), then we are going to return pre-maturely Instead, we should probably (try to) forward to next container and look for patches there as well.

Shall fix this logic in a v2..


BTW, it may be worth documenting container format in English. As it stands now, one has to look at the code to figure out the layout.

-boris


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.