public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* static data in dxe_runtime modules
@ 2019-08-01 19:16 Roman Kagan
  2019-08-02 23:54 ` [edk2-devel] " Andrew Fish
  2019-08-03  2:03 ` Laszlo Ersek
  0 siblings, 2 replies; 15+ messages in thread
From: Roman Kagan @ 2019-08-01 19:16 UTC (permalink / raw)
  To: devel@edk2.groups.io

I'm trying to come up with a solution to the problem that OpenSSL
internally uses static variables ("per-thread" in no-threads config) to
store pointers, which remain unadjusted upon SetVirtualAddressMap and
cause the guest OS crash.

More specifically, trying to set a signed variable leads to the
following call chain:

VariableServiceSetVariable
  ProcessVarWithPk
    VerifyTimeBasedPayloadAndUpdate
      Pkcs7GetSigners
	ASN1_item_d2i
	  asn1_item_embed_d2i
	    asn1_template_ex_d2i
	      asn1_template_noexp_d2i
		asn1_item_embed_d2i
		  asn1_template_ex_d2i
		    asn1_template_noexp_d2i
		      asn1_item_embed_d2i
			asn1_template_ex_d2i
			  asn1_template_noexp_d2i
			    asn1_item_embed_d2i
			      pubkey_cb
				ERR_set_mark
				  ERR_get_state

The latter performs one-time initialization if needed, and then returns
a pointer maintained in a static array using CRYPTO_THREAD_get_local().
If a signed variable is set before SetVirtualAddressMap and (another
one) after it, the pointer is initialized while in the old mapping and
dereferenced while in the new one, crashing the OS.

The real-world reproducer: install Windows Server 2016 in a VM with
OVMF, shut it down, replace the varstore with a fresh template copy,
boot Windows, try to update, say, "dbx" from within Windows => BSOD.
The reason is that the Windows bootloader stores a secure variable
"CurrentPolicy" if it isn't there, triggering the above callchain while
still in identity mapping.
(This problem isn't widely noticed because during the installation the
bootloader stores CurrentPolicy, but the OS is soon rebooted, before it
attempts to update dbx; upon that CurrentPolicy remains in the varstore
and on further boots the bootloader skips setting it.)

What would be the best way to fix it?

One option is to audit OpenSSL and make sure it either doesn't put
pointers into static storage or properly cleans them up (and call the
cleanup function in RuntimeCryptLibAddressChangeEvent); then assume the
rest of EDKII code is safe in this regard.

Another is to assume that no static data in dxe_runtime modules should
survive SetVirtualAddressMap, and thus make
PeCoffLoaderRelocateImageForRuntime reinitialize the modules from
scratch instead of re-applying the relocations only.

I must admit I don't yet quite understand the full consequences of
either option.  Perhaps there are better ones.
Any suggestion is appreciated.

Thanks,
Roman.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2019-08-16 17:00 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-01 19:16 static data in dxe_runtime modules Roman Kagan
2019-08-02 23:54 ` [edk2-devel] " Andrew Fish
2019-08-03  2:03 ` Laszlo Ersek
2019-08-05 10:18   ` Roman Kagan
2019-08-07 17:29     ` Laszlo Ersek
2019-08-07 17:41       ` Andrew Fish
2019-08-08 17:39         ` Laszlo Ersek
2019-08-09 16:07           ` Roman Kagan
     [not found]           ` <15B94CD6CF07DEE2.13696@groups.io>
2019-08-12 18:43             ` Roman Kagan
2019-08-13  9:10               ` Laszlo Ersek
2019-08-13 11:23                 ` Roman Kagan
2019-08-14 15:16                   ` Laszlo Ersek
2019-08-14 16:26                     ` Andrew Fish
2019-08-16 15:22                       ` Laszlo Ersek
2019-08-16 17:00                       ` Roman Kagan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox