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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.01.18 at 17:53, <tamas@xxxxxxxxxxxxx> wrote:
> On Wed, Jan 3, 2018 at 9:36 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>> On 03.01.18 at 17:04, <tamas@xxxxxxxxxxxxx> wrote:
>>> On Wed, Jan 3, 2018 at 4:20 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>>> On 02.01.18 at 16:56, <tamas@xxxxxxxxxxxxx> wrote:
>>>>> +        if ( elo->Attributes & LOAD_OPTION_ACTIVE )
>>>>
>>>> Without any other (earlier) check, how can you reliably tell this
>>>> being a pointer to EFI_LOAD_OPTION from it being the
>>>> currently used one to CHAR16? Is the distinction perhaps UEFI
>>>> version dependent? In the 2.3 spec I can't find any information
>>>> on the layout of what ->LoadOptions points to.
>>>
>>> AFAICT there is no clear cut way to distinguish what is being passed
>>> here. When launched via UEFI Xen receives the EFI_LOAD_OPTION buffer
>>> here already, which it tries to parse as a string and fails. Take a
>>> look at the same problem in the shim:
>>> https://github.com/rhboot/shim/blob/master/shim.c#L2501. If there is a
>>> clearer way to distinguish what is being passed here or more checks
>>> that can be done I would be open for suggestions.
>>
>> Well, first of all the code you point to tells me that this situation is
>> indeed as bad as it can be. However, the code also shows you
>> some heuristics you could re-use.
> 
> From what I've seen the only heuristic we could copy is counting ucs2
> strings in the buffer. I wasn't entirely convinced that it's necessary
> and rather went with the "if it looks like a duck, swims like a duck,
> and quacks like a duck, then it probably is a duck" test. What are the
> chances we could encounter a string that also properly parses as an
> EFI_LOAD_OPTION buffer passing the checks in place in this patch?
> Anyway, if the ucs2 string counting is something we also want to
> utilize, I have nothing against it.

Actually that string counting seemed rather fragile, but maybe I
didn't fully understand it. 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.

Jan


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