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

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

>>> 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.


Xen-devel mailing list



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