Re: [PATCH 1/5] OvmfPkg/VirtioNetDxe: map VRING using VirtioRingMap()

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 5 September 2017 at 23:37, Laszlo Ersek <lersek@redhat.com> wrote:
> On 09/06/17 00:18, Ard Biesheuvel wrote:
>> On 5 September 2017 at 22:59, Laszlo Ersek <lersek@redhat.com> wrote:
>>> On 09/05/17 23:11, Ard Biesheuvel wrote:
>
>>>> (which means it could be the second time it was called).
>>>
>>> Side remark: the CoreExitBootServices() implementation does not notice
>>> memory map changes incurred by the ExitBootServices() handlers, in my
>>> interpretation.
>>>
>>> CoreExitBootServices() has a MapKey (= "memmap generation") check early
>>> on, in CoreTerminateMemoryMap(). This check catches memmap changes
>>> between the last GetMemoryMap(), and the call to ExitBootServices().
>>>
>>> After this check succeeds, the notify functions are kicked, and on this
>>> path, CoreExitBootServices() simply cannot return any other value than
>>> EFI_SUCCESS. So whatever mess the individual callbacks make, it doesn't
>>> notice or report.
>>>
>>
>> CoreExitBootServices() disables the timer first, and so it will return
>> with event dispatch disabled if it fails, ensuring that it is no
>> longer possible for an event handler to be invoked between
>> GetMemoryMap and ExitBootServices.
>
> True, but that's not what I meant -- I meant that, if an EBS handler
> violates its contract and changes the memory map (= "it makes mess"), on
> the call stack of
>
>   CoreNotifySignalList (&gEfiEventExitBootServicesGuid);
>
> then CoreExitBootServices() won't notice it, it won't return an error,
> and the caller of EBS() won't go back to calling GetMemoryMap() again.
>

Ah right. So the only thing the memory map key protects against is
inadvertent interruptions by event handlers that modify the memory map
after GetMemoryMap(). It does not protect against programming errors.