Re: [Xen-devel] [PATCH] xen: Add EFI_LOAD_OPTION support

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.01.18 at 15:39, <tamas@xxxxxxxxxxxxx> wrote:
> On Thu, Jan 4, 2018 at 3:43 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>> Just looking at the low bit of the first
>> byte before assuming this could be a load option structure,
>> however, is too weak a check for my taste.
> 
> There is more done in this patch then just checking the low bit of the
> Attributes field: calculating where the OptionalData should be if this
> is indeed an EFI_LOAD_OPTION and then sanity checking if that address
> is sane or not. This requires the buffer to have a layout that at
> least parses properly as an EFI_LOAD_OPTION, hence it likely is one..

But that calculation is not doing all the checking that would be
needed to be reasonably safe. In particular, you mustn't call
wstrlen() when you don't know whether all of the supposed
string is actually inside the given memory block size. And I'm
also not entirely certain whether caring of overflow in only one
(late) place is sufficient.

Btw, looking at the code again, I see two more cosmetic things
for you to change: Please use sizeof(CHAR16) both in place of
sizeof(L'\0') and instead of the literal 2 there.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel