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

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Thu, Jan 4, 2018 at 8:00 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> 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.

Fair enough. How about if we check whether the supposed string start
spot is within the buffer and then use wstrnlen in place such that it
can't go past the buffer? Would that be enough sanity checking?

Tamas

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