Re: [edk2-devel] static data in dxe_runtime modules

["Andrew Fish" <afish@apple.com>]

[-- Attachment #1: Type: text/plain, Size: 3958 bytes --]



> On Aug 14, 2019, at 8:16 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> 
> On 08/13/19 13:23, Roman Kagan wrote:
>> On Tue, Aug 13, 2019 at 11:10:27AM +0200, Laszlo Ersek wrote:
>>> On 08/12/19 20:43, Roman Kagan wrote:
>>>> On Fri, Aug 09, 2019 at 04:07:00PM +0000, Roman Kagan via Groups.Io wrote:
>>>>> On Thu, Aug 08, 2019 at 07:39:14PM +0200, Laszlo Ersek wrote:
>>>>>> On 08/07/19 19:41, Andrew Fish wrote:
>>>>>>>> On Aug 7, 2019, at 10:29 AM, Laszlo Ersek <lersek@redhat.com> wrote:
>>>>>>>> On 08/05/19 12:18, Roman Kagan wrote:
>>>>>>>>> On Sat, Aug 03, 2019 at 04:03:04AM +0200, Laszlo Ersek via Groups.Io wrote:
>>>>>>>>>> On 08/01/19 21:16, Roman Kagan wrote:
>>>>>>>> I'm convinced that OpenSSL needs to expose a new API for this particular
>>>>>>>> problem.
>>>>> 
>>>>> Since, as you point out below, the problem only affects the essentially
>>>>> broken configuration (SECURE_BOOT_ENABLE && !SMM_REQUIRE), I'm fine with
>>>>> saving time and effort and sticking to the hack-ish approach proposed in
>>>>> the bugzilla issue, which is to iterate over "thread-local" pointers and
>>>>> EfiConvertPointer() on each.  (As long as it fixes the problem of
>>>>> course; I'll test and report back.)
>>>> 
>>>> It doesn't :(  It just gets slightly further and hits another static
>>>> pointer variable which is not part of the thread-local array:
>>>> 
>>>> ...
>>>>  Pkcs7Verify
>>>>    EVP_add_digest
>>>>      OBJ_NAME_add
>>>> 
>>>> this one uses a few static pointer variables that are also initialized
>>>> on demand and become stale upon SetVirtualAddressMap().
>>> 
>>> So it looks like the issue can't be solved without making OpenSSL aware
>>> of this use case.
>> 
>> Is reloading the module from scratch ruled out completely?
> 
> Not my place to say authoritatively, but:
> - it would be a first, as much as I can say,
> - it would duplicate (in purpose) an existing facility.
> 
> Personally I'd expect it to be rejected, but it's not up to me. If
> you're willing to "build one to (possibly) throw away", that could be
> the most direct way to get authoritative (= maintainer) feedback.
> 

Laszlo,

I thunk it is more likely to get rejected as it would not work. 

Every runtime driver I've every seen usually works like this:
1) Loads as an EFI driver and uses EFI Boot Services in its constructor (gBS, gDS, AllocatePool(), etc.)
2) You use the EFI Boot Service to register the ExitBootServices Event. 
3) SetVirtualAddressMap event fires and converts all the pointers 
4) After all the ExitBootServices events have been processed the DXE Runtime driver re-relocates the Runtime Driver
5) The next code that runs is a call from the kernel virtual mapping, and the system is at runtime 

It is important to remember that when gRT->SetVirtualAddressMap() is called by the OS Loader (or early kernel) that gBS->ExitBootServices() has already been called. So by the time you get 3) almost all of EFI is gone. The only services that remain are gRT. Note you find the location of gRT by using the gST passed into the entry point of the driver. So here is lies the problem the entry point is passed a Handle (EFI Boot Services concept) and a pointer to gST (another boot services table). So you can't really reload a module and expect it to work. 

In EFI the transition from Boot Service time to Runtime already requires a lot of coding discipline to not call services at runtime that will go away after ExitBootServices. Having C code that can be called in Physical mode, and then with a virtual mapping is a very unnatural and what EFI does today is the minimum required to make things work. 

Also remember dumping more into EFI runtime is stealing memory and usually kernel virtual address space from the OS. 

Thanks,

Andrew Fish


> Thanks
> Laszlo
> 
>> I'd try to cook up a patch for that unless there's a strong no-go.
>> 
>> Thanks,
>> Roman.


[-- Attachment #2: Type: text/html, Size: 13808 bytes --]