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.

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#44831): https://edk2.groups.io/g/devel/message/44831
Mute This Topic: https://groups.io/mt/32686575/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to