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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.01.18 at 17:16, <tamas@xxxxxxxxxxxxx> wrote:
> 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?

That should be enough, yes. Do we have wstrnlen() though?
Rather than introducing it, open-coding the search for the nul
terminator may be more appropriate in a case like this one.

Jan


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