From: Michael Kelley <mikelley@microsoft.com> To: Ard Biesheuvel <ardb@kernel.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org> Cc: "linux-efi@vger.kernel.org" <linux-efi@vger.kernel.org>, "leif@nuviainc.com" <leif@nuviainc.com>, Boqun Feng <Boqun.Feng@microsoft.com> Subject: RE: [PATCH] efi/libstub/arm64: avoid image_base value from efi_loaded_image Date: Sat, 28 Mar 2020 23:55:20 +0000 [thread overview] Message-ID: <MW2PR2101MB10525BAFA3880DD27216C4DCD7CD0@MW2PR2101MB1052.namprd21.prod.outlook.com> (raw) In-Reply-To: <20200328205809.23825-1-ardb@kernel.org> From: Ard Biesheuvel <ardb@kernel.org> Sent: Saturday, March 28, 2020 1:58 PM > > Commit 9f9223778ef3 ("efi/libstub/arm: Make efi_entry() an ordinary > PE/COFF entrypoint") did some code refactoring to get rid of the > EFI entry point assembler code, and in the process, it got rid of the > assignment of image_addr to the value of _text. Instead, it switched > to using the image_base field of the efi_loaded_image struct provided > by UEFI, which should contain the same value. > > However, Michael reports that this is not the case: older GRUB builds > corrupt this value in some way, and since we can easily switch back to > referring to _text to discover this value, let's simply do that. > > While at it, fix another issue in commit 9f9223778ef3, which may result > in the unassigned image_addr to be misidentified as the preferred load > offset of the kernel, which is unlikely but will cause a boot crash if > it does occur. > > Finally, let's add a warning if the _text vs. image_base discrepancy is > detected, so we can tell more easily how widespread this issue actually > is. > > Reported-by: Michael Kelley <mikelley@microsoft.com> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > --- > drivers/firmware/efi/libstub/arm64-stub.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) Tested in a Hyper-V VM. With the 2.02~beta2 version of grub, the FIRMWARE BUG message is output and then Linux correctly boots. With the 2.04-4 version of grub, Linux correctly boots with no error messages. Tested-by: Michael Kelley <mikelley@microsoft.com> > > diff --git a/drivers/firmware/efi/libstub/arm64-stub.c > b/drivers/firmware/efi/libstub/arm64-stub.c > index 9254cd8ab2d3..db0c1a9c1699 100644 > --- a/drivers/firmware/efi/libstub/arm64-stub.c > +++ b/drivers/firmware/efi/libstub/arm64-stub.c > @@ -116,6 +116,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr, > * Mustang), we can still place the kernel at the address > * 'dram_base + TEXT_OFFSET'. > */ > + *image_addr = (unsigned long)_text; > if (*image_addr == preferred_offset) > return EFI_SUCCESS; > > @@ -140,7 +141,11 @@ efi_status_t handle_kernel_image(unsigned long *image_addr, > } > *image_addr = *reserve_addr + TEXT_OFFSET; > } > - memcpy((void *)*image_addr, image->image_base, kernel_size); > + > + if (image->image_base != _text) > + pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus > value\n"); > + > + memcpy((void *)*image_addr, _text, kernel_size); > > return EFI_SUCCESS; > } > -- > 2.17.1
WARNING: multiple messages have this Message-ID (diff)
From: Michael Kelley <mikelley@microsoft.com> To: Ard Biesheuvel <ardb@kernel.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org> Cc: Boqun Feng <Boqun.Feng@microsoft.com>, "leif@nuviainc.com" <leif@nuviainc.com>, "linux-efi@vger.kernel.org" <linux-efi@vger.kernel.org> Subject: RE: [PATCH] efi/libstub/arm64: avoid image_base value from efi_loaded_image Date: Sat, 28 Mar 2020 23:55:20 +0000 [thread overview] Message-ID: <MW2PR2101MB10525BAFA3880DD27216C4DCD7CD0@MW2PR2101MB1052.namprd21.prod.outlook.com> (raw) In-Reply-To: <20200328205809.23825-1-ardb@kernel.org> From: Ard Biesheuvel <ardb@kernel.org> Sent: Saturday, March 28, 2020 1:58 PM > > Commit 9f9223778ef3 ("efi/libstub/arm: Make efi_entry() an ordinary > PE/COFF entrypoint") did some code refactoring to get rid of the > EFI entry point assembler code, and in the process, it got rid of the > assignment of image_addr to the value of _text. Instead, it switched > to using the image_base field of the efi_loaded_image struct provided > by UEFI, which should contain the same value. > > However, Michael reports that this is not the case: older GRUB builds > corrupt this value in some way, and since we can easily switch back to > referring to _text to discover this value, let's simply do that. > > While at it, fix another issue in commit 9f9223778ef3, which may result > in the unassigned image_addr to be misidentified as the preferred load > offset of the kernel, which is unlikely but will cause a boot crash if > it does occur. > > Finally, let's add a warning if the _text vs. image_base discrepancy is > detected, so we can tell more easily how widespread this issue actually > is. > > Reported-by: Michael Kelley <mikelley@microsoft.com> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > --- > drivers/firmware/efi/libstub/arm64-stub.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) Tested in a Hyper-V VM. With the 2.02~beta2 version of grub, the FIRMWARE BUG message is output and then Linux correctly boots. With the 2.04-4 version of grub, Linux correctly boots with no error messages. Tested-by: Michael Kelley <mikelley@microsoft.com> > > diff --git a/drivers/firmware/efi/libstub/arm64-stub.c > b/drivers/firmware/efi/libstub/arm64-stub.c > index 9254cd8ab2d3..db0c1a9c1699 100644 > --- a/drivers/firmware/efi/libstub/arm64-stub.c > +++ b/drivers/firmware/efi/libstub/arm64-stub.c > @@ -116,6 +116,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr, > * Mustang), we can still place the kernel at the address > * 'dram_base + TEXT_OFFSET'. > */ > + *image_addr = (unsigned long)_text; > if (*image_addr == preferred_offset) > return EFI_SUCCESS; > > @@ -140,7 +141,11 @@ efi_status_t handle_kernel_image(unsigned long *image_addr, > } > *image_addr = *reserve_addr + TEXT_OFFSET; > } > - memcpy((void *)*image_addr, image->image_base, kernel_size); > + > + if (image->image_base != _text) > + pr_efi_err("FIRMWARE BUG: efi_loaded_image_t::image_base has bogus > value\n"); > + > + memcpy((void *)*image_addr, _text, kernel_size); > > return EFI_SUCCESS; > } > -- > 2.17.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-03-28 23:55 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-03-28 20:58 [PATCH] efi/libstub/arm64: avoid image_base value from efi_loaded_image Ard Biesheuvel 2020-03-28 20:58 ` Ard Biesheuvel 2020-03-28 23:55 ` Michael Kelley [this message] 2020-03-28 23:55 ` Michael Kelley 2020-03-30 7:47 ` Leif Lindholm 2020-03-30 7:47 ` Leif Lindholm 2020-03-30 7:50 ` Ard Biesheuvel 2020-03-30 7:50 ` Ard Biesheuvel 2020-03-30 7:51 ` Ard Biesheuvel 2020-03-30 7:51 ` Ard Biesheuvel 2020-03-30 18:12 ` Michael Kelley 2020-03-30 18:12 ` Michael Kelley 2020-03-30 18:24 ` Ard Biesheuvel 2020-03-30 18:24 ` Ard Biesheuvel 2020-03-31 7:56 ` Ard Biesheuvel 2020-03-31 7:56 ` Ard Biesheuvel 2020-03-31 13:37 ` Michael Kelley 2020-03-31 13:37 ` Michael Kelley 2020-04-06 17:13 ` Michael Kelley 2020-04-06 17:13 ` Michael Kelley 2020-04-07 8:07 ` Ard Biesheuvel 2020-04-07 8:07 ` Ard Biesheuvel 2020-03-31 19:20 ` Laszlo Ersek 2020-03-31 19:20 ` Laszlo Ersek 2020-04-01 8:24 ` Ard Biesheuvel 2020-04-01 8:24 ` Ard Biesheuvel
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-all 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=MW2PR2101MB10525BAFA3880DD27216C4DCD7CD0@MW2PR2101MB1052.namprd21.prod.outlook.com \ --to=mikelley@microsoft.com \ --cc=Boqun.Feng@microsoft.com \ --cc=ardb@kernel.org \ --cc=leif@nuviainc.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-efi@vger.kernel.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.