From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@apple.com header.s=20180706 header.b=CG7HToSU; spf=pass (domain: apple.com, ip: 17.171.2.68, mailfrom: afish@apple.com) Received: from ma1-aaemail-dr-lapp02.apple.com (ma1-aaemail-dr-lapp02.apple.com [17.171.2.68]) by groups.io with SMTP; Wed, 14 Aug 2019 09:26:51 -0700 Received: from pps.filterd (ma1-aaemail-dr-lapp02.apple.com [127.0.0.1]) by ma1-aaemail-dr-lapp02.apple.com (8.16.0.27/8.16.0.27) with SMTP id x7EGC3iT063783; Wed, 14 Aug 2019 09:26:49 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=sender : from : message-id : content-type : mime-version : subject : date : in-reply-to : cc : to : references; s=20180706; bh=hnZ+CopASBPpqiS4m2dK1uy4yvYR+E8xdJsZMuUkfqE=; b=CG7HToSUcv2fqFYWN47IIvJbEFg1gnh27xBAiC0j8jeBIXknu2nBB1iwbg7zhmkoUwUn gZTo0aYz5edfgbpzVOmYwJ0yljgOGbei+JmxxDvZGIRxxkV9MVi9ZlWYRAUmJP0uXwcn dPXkEthnCUrl6GwzxxA3+HNPq+iYIJx30xRBho277s+/fLA1hRygTvNFrHHIhopVnbAG QTWJLCBYF692L7z4zek8+f7csiYa8IOosL7NpZ7AmT1wdOVkivKKAVWMAhrsMcO48DUe FTtdlLuYfT9ajle7kfSLYR5f6qLIGN1Ytb/Mu4IwWI0SvH4ULl75rm9sdUjTz2slkgEz JQ== Received: from mr2-mtap-s01.rno.apple.com (mr2-mtap-s01.rno.apple.com [17.179.226.133]) by ma1-aaemail-dr-lapp02.apple.com with ESMTP id 2u9tm24a98-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Wed, 14 Aug 2019 09:26:49 -0700 Received: from nwk-mmpp-sz09.apple.com (nwk-mmpp-sz09.apple.com [17.128.115.80]) by mr2-mtap-s01.rno.apple.com (Oracle Communications Messaging Server 8.0.2.4.20190507 64bit (built May 7 2019)) with ESMTPS id <0PW800EUOJ0OQDD0@mr2-mtap-s01.rno.apple.com>; Wed, 14 Aug 2019 09:26:49 -0700 (PDT) Received: from process_milters-daemon.nwk-mmpp-sz09.apple.com by nwk-mmpp-sz09.apple.com (Oracle Communications Messaging Server 8.0.2.4.20190507 64bit (built May 7 2019)) id <0PW800200IYPRZ00@nwk-mmpp-sz09.apple.com>; Wed, 14 Aug 2019 09:26:49 -0700 (PDT) X-Va-A: X-Va-T-CD: 07a9f7dd315dc6000695a0402a47d12d X-Va-E-CD: d3bad307e4af6c7675d5029cab479fa4 X-Va-R-CD: 2fcd58ab292acf25a8ea4fedd159c548 X-Va-CD: 0 X-Va-ID: 5cdfda11-6282-4b8c-8b8f-f0d0e32c5f56 X-V-A: X-V-T-CD: 07a9f7dd315dc6000695a0402a47d12d X-V-E-CD: d3bad307e4af6c7675d5029cab479fa4 X-V-R-CD: 2fcd58ab292acf25a8ea4fedd159c548 X-V-CD: 0 X-V-ID: 838b8dc8-3145-404b-8293-0df3ddc9edfd X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2019-08-14_06:,, signatures=0 Received: from [17.235.54.168] (unknown [17.235.54.168]) by nwk-mmpp-sz09.apple.com (Oracle Communications Messaging Server 8.0.2.4.20190507 64bit (built May 7 2019)) with ESMTPSA id <0PW800G2QJ0NOM70@nwk-mmpp-sz09.apple.com>; Wed, 14 Aug 2019 09:26:48 -0700 (PDT) Sender: afish@apple.com From: "Andrew Fish" Message-id: MIME-version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\)) Subject: Re: [edk2-devel] static data in dxe_runtime modules Date: Wed, 14 Aug 2019 09:26:47 -0700 In-reply-to: Cc: Roman Kagan , devel@edk2.groups.io, David Woodhouse To: Laszlo Ersek References: <20190801191621.GB14235@rkaganb.sw.ru> <8d18d4f6-5f33-44e9-2758-46350b43c5ec@redhat.com> <20190805101813.GA27171@rkaganb.sw.ru> <406f2250-41e2-9925-b570-38b99a5f6e41@redhat.com> <0A900AFC-C9A0-4A4C-8EBA-9A6F75B3EE25@apple.com> <5d03c05d-24c2-f825-c42e-4371a87d76a1@redhat.com> <15B94CD6CF07DEE2.13696@groups.io> <20190812184303.GA4212@rkaganb.sw.ru> <62712b07-31c1-a19f-1660-12da0d7bac50@redhat.com> <20190813112308.GB4212@rkaganb.sw.ru> X-Mailer: Apple Mail (2.3445.104.11) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2019-08-14_06:,, signatures=0 Content-type: multipart/alternative; boundary="Apple-Mail=_01E6423F-30B8-4772-8828-8A8A3B4281DA" --Apple-Mail=_01E6423F-30B8-4772-8828-8A8A3B4281DA Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii > On Aug 14, 2019, at 8:16 AM, Laszlo Ersek wrote: >=20 > 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 = 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. >>>>>=20 >>>>> 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.) >>>>=20 >>>> It doesn't :( It just gets slightly further and hits another = static >>>> pointer variable which is not part of the thread-local array: >>>>=20 >>>> ... >>>> Pkcs7Verify >>>> EVP_add_digest >>>> OBJ_NAME_add >>>>=20 >>>> this one uses a few static pointer variables that are also = initialized >>>> on demand and become stale upon SetVirtualAddressMap(). >>>=20 >>> So it looks like the issue can't be solved without making OpenSSL = aware >>> of this use case. >>=20 >> Is reloading the module from scratch ruled out completely? >=20 > 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. >=20 > 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 (=3D maintainer) feedback. >=20 Laszlo, I thunk it is more likely to get rejected as it would not work.=20 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.=20= 3) SetVirtualAddressMap event fires and converts all the pointers=20 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=20 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.=20 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.=20 Also remember dumping more into EFI runtime is stealing memory and = usually kernel virtual address space from the OS.=20 Thanks, Andrew Fish > Thanks > Laszlo >=20 >> I'd try to cook up a patch for that unless there's a strong no-go. >>=20 >> Thanks, >> Roman. --Apple-Mail=_01E6423F-30B8-4772-8828-8A8A3B4281DA Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=us-ascii

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 (=3D 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.

= --Apple-Mail=_01E6423F-30B8-4772-8828-8A8A3B4281DA--