public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Andrew Fish <afish@apple.com>
To: edk2-devel <edk2-devel@lists.01.org>
Subject: [MdeModulePkg] SetVirtualAddressMap() crashed due to DxeReportStatusCodeLib assuming the state of the BootService Memory at runtime.
Date: Mon, 08 Aug 2016 16:07:45 -0700	[thread overview]
Message-ID: <776695E1-E864-42F4-A633-B86FB913A510@apple.com> (raw)

I was messing about with an ExitBootServices test that fills boot services memory with 0xAFAFAFAFAFAFAFAF (It was Vincent's idea to use my Initials but it has the handy property of being a non-cononical address and causes on GP fault on X64) and SetVirtualAddressMap() started crashing. 

It looks like this code is assuming the 1st call to ReportStatus code will not happen at runtime. This is not the case for the RuntimeDxe driver. 
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c#L43
VOID
InternalGetReportStatusCode (
  VOID
  )
{
  EFI_STATUS  Status;

  if (mReportStatusCodeLibStatusCodeProtocol != NULL) {
    return;
  }

  //
  // Check gBS just in case ReportStatusCode is called before gBS is initialized.
  //
  if (gBS != NULL && gBS->LocateProtocol != NULL) {
    Status = gBS->LocateProtocol (&gEfiStatusCodeRuntimeProtocolGuid, NULL, (VOID**) &mReportStatusCodeLibStatusCodeProtocol);
    if (EFI_ERROR (Status)) {
      mReportStatusCodeLibStatusCodeProtocol = NULL;
    }
  }
}

I'm guessing this seems to work due to:https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c#L803

  //
  // Zero out the Boot Service Table
  //
  ZeroMem (gBS, sizeof (EFI_BOOT_SERVICES));


Thus if I'm looking at this code correctly it only looks like it works at Runtime since it is depending on the value of a boot services memory buffer not changing. This is not a valid assumption as that code is owned by the caller of ExitBootServices, so it should be legal for my test to change the value. 

I wanted to get a few more eyes on this prior to filling a bug?

Thanks,

Andrew Fish




             reply	other threads:[~2016-08-08 23:07 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-08 23:07 Andrew Fish [this message]
2016-08-09  1:21 ` [MdeModulePkg] SetVirtualAddressMap() crashed due to DxeReportStatusCodeLib assuming the state of the BootService Memory at runtime Zeng, Star
2016-08-09  2:07   ` Andrew Fish
2016-08-09  2:26     ` Zeng, Star
2016-08-09 14:50       ` Andrew Fish
2016-08-10 17:22         ` Kinney, Michael D
2016-08-10 17:35           ` Andrew Fish
2016-08-10 18:02             ` Kinney, Michael D
2016-08-10 19:04               ` Andrew Fish
2016-08-11  1:53                 ` Zeng, Star
2016-08-11  1:59                   ` Andrew Fish
2016-08-11  3:06                     ` Zeng, Star

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=776695E1-E864-42F4-A633-B86FB913A510@apple.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox