Ovmf loadimage startimage v3 #424

ardbiesheuvel · 2020-03-05T19:42:33Z

No description provided.

In an upcoming patch, we will introduce a separate DXE driver that exposes the virtual SimpleFileSystem implementation that carries the kernel and initrd passed via the QEMU command line, and a separate library that consumes it, to be incorporated into the boot manager. Since the GUID used for the SimpleFileSystem implementation's device path will no longer be for internal use only, create a well defined GUID to identify the media device path. Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Expose the existing implementation of an abstract filesystem exposing the blobs passed to QEMU via the command line via a standalone DXE driver. Notable difference with the original code is the switch to a new vendor GUIDed media device path, as opposed to a vendor GUID hardware device path, which is not entirely appropriate for pure software constructs. Since we are using the GetTime() runtime service in a DXE_DRIVER type module, we need to DEPEX explicitly on gEfiRealTimeClockArchProtocolGuid. Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Introduce the QemuLoadImageLib library class that we will instantiate to load the kernel image passed via the QEMU command line using the standard LoadImage boot service. Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Implement QemuLoadImageLib, and make it load the image provided by the QEMU_EFI_LOADER_FS_MEDIA_GUID/kernel device path that we implemented in a preceding patch in a separate DXE driver, using only the standard LoadImage and StartImage boot services. Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Add the QEMU loader DXE driver and client library to the build for our QEMU targeted implementations in ArmVirtPkg. Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Drop the QEMU loader file system implementation inside this library, and switch to the separate QemuLoadImageLib library and the associated driver to expose the kernel and initrd passed via the QEMU command line. Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

We have no need for exposing the kernel command line as a file, so remove support for that. Since the remaining blobs (kernel and initrd) are typically much larger than a page, switch to the page based allocator for blobs at the same time. Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

On x86, the kernel image consists of a setup block and the actual kernel, and QEMU presents these as separate blobs, whereas on disk (and in terms of PE/COFF image signing), they consist of a single image. So add support to our FS loader driver to expose files via the abstract file system that consist of up to two concatenated blobs, and redefine the kernel file so it consists of the setup and kernel blobs, on every architecture (on non-x86, the setup block is simply 0 bytes and is therefore ignored implicitly) Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

In preparation of moving the legacy x86 loading to an implementation of the QEMU load image library class, introduce a protocol header and GUID that we will use to identify legacy loaded x86 Linux kernels in the protocol database. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Replace the open coded sequence to load Linux on x86 with a short and generic sequence invoking QemuLoadImageLib, which can be provided by a generic version that only supports the LoadImage and StartImage boot services, and one that incorporates the entire legacy loading sequence as well. Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

…e path Linux v5.7 will introduce a new method to load the initial ramdisk (initrd) from the loader, using the LoadFile2 protocol installed on a special vendor GUIDed media device path. Add support for this to our QEMU command line kernel/initrd loader. Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

The QemuLoadImageLib implementation we currently use for all OVMF builds copies the behavior of the QEMU loader code that precedes it, which is to disregard UEFI secure boot policies entirely when it comes to loading kernel images that have been specified on the QEMU command line. This behavior deviates from ArmVirtQemu based builds, which do take UEFI secure boot policies into account, and refuse to load images from the command line that cannot be authenticated. The disparity was originally due to the fact that the QEMU command line kernel loader did not use LoadImage and StartImage at all, but this changed recently, and now, there are only a couple of reasons left to stick with the legacy loader: - it permits loading images that lack a valid PE/COFF header, - it permits loading X64 kernels on IA32 firmware running on a X64 capable system. Since every non-authentic PE/COFF image can trivially be converted into an image that lacks a valid PE/COFF header, the former case can simply not be supported in a UEFI secure boot context. The latter case is highly theoretical, given that one could easily switch to native X64 firmware in a VM scenario. That leaves us with little justification to use the legacy loader at all when UEFI secure boot policies are in effect, so let's switch to the generic loader for UEFI secure boot enabled builds. Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Implement another version of QemuLoadImageLib that uses LoadImage and StartImage, but falls back to the legacy Linux loader code if that fails. The logic in the legacy fallback routines is identical to the current QEMU linux loader for X64 and IA32. Note the use of the OVMF_LOADED_X86_LINUX_KERNEL protocol for the legacy loaded image: this makes it possible to expose the LoadImage/StartImage abstraction for the legacy loader, using the EFI paradigm of identifying a loaded image solely by a handle. Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Add the components that expose the QEMU abstract loader file system so that we can switch over our PlatformBmLib over to it in a subsequent patch. Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2566 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

…bSfs (tianocore#424) ## Description GetCacheFileName() may return NULL. Add a check before passing the return value to ShellOpenFileByName(). - [ ] Impacts functionality? - **Functionality** - Does the change ultimately impact how firmware functions? - Examples: Add a new library, publish a new PPI, update an algorithm, ... - [x] Impacts security? - **Security** - Does the change have a direct security impact on an application, flow, or firmware? - Examples: Crypto algorithm change, buffer overflow fix, parameter validation improvement, ... - [ ] Breaking change? - **Breaking change** - Will anyone consuming this change experience a break in build or boot behavior? - Examples: Add a new library class, move a module to a different repo, call a function in a new library class in a pre-existing module, ... - [ ] Includes tests? - **Tests** - Does the change include any explicit test code? - Examples: Unit tests, integration tests, robot tests, ... - [ ] Includes documentation? - **Documentation** - Does the change contain explicit documentation additions outside direct code modifications (and comments)? - Examples: Update readme file, add feature readme file, link to documentation on an a separate Web page, ... ## How This Was Tested Booting to shell on Q35 ## Integration Instructions N/A

When converting ELF to PE/COFF for the AArch64 target, we may encounter an R_AARCH64_ADR_GOT_PAGE relocation that refers to an ADR instruction instead of an ADRP instruction. This can happen when the toolchain is working around Cortex-A53 erratum #843419. If that's the case, be sure to calculate the offset appropriately. This resolves an issue experienced when building a StandaloneMm image (which is built with -fpie) with stack protection enabled on GCC compiled with "--enable-fix-cortex-a53-843419". In this case, the linker may convert an ADRP instruction appearing at an offset of 0xff8 or 0xffc modulo 4KiB into an ADR instruction, but will leave the original R_AARCH64_ADR_GOT_PAGE relocation in place. (This is not a bug in the linker, given that there is no other relocation type that it could reasonably convert it into) In this scenario, the following code is being generated by the toolchain: # Load to set the stack canary 2ffc: 10028020 adr x0, 8000 <mErrorString+0x1bc> 3008: f940d400 ldr x0, [x0, tianocore#424] # Load to check the stack canary 30cc: b0000020 adrp x0, 8000 <mErrorString+0x1bc> 30d0: f940d400 ldr x0, [x0, tianocore#424] GenFw rewrote that to: # Load to set the stack canary 2ffc: 10000480 adr x0, 0x308c 3008: 912ec000 add x0, x0, #0xbb0 # Load to check the stack canary 30cc: f0000460 adrp x0, 0x92000 30d0: 912ec000 add x0, x0, #0xbb0 Note that we're now setting the stack canary from the wrong address, resulting in an erroneous stack fault. After this fix, the offset will be calculated correctly for an ADR and the stack canary is set correctly. Note that there is a corner case where this may cause the conversion to fail: if the original GOT entry is just within -/+ 1 MiB of the reference, but the actual variable it refers to is not, the resulting offset cannot be represented by the immediate offset field in a ADR instruction. Given that this issue only affects PIE executables, which are rare and usually tiny, this is unlikely to cause problems in practice. Ref: https://edk2.groups.io/g/devel/topic/102202314 [ardb: expand commit log, add reference] Signed-off-by: Jake Garver <jake@nvidia.com> Reviewed-by: Rebecca Cran <rebecca@bsdio.com>

When converting ELF to PE/COFF for the AArch64 target, we may encounter an R_AARCH64_ADR_GOT_PAGE relocation that refers to an ADR instruction instead of an ADRP instruction. This can happen when the toolchain is working around Cortex-A53 erratum #843419. If that's the case, be sure to calculate the offset appropriately. This resolves an issue experienced when building a StandaloneMm image (which is built with -fpie) with stack protection enabled on GCC compiled with "--enable-fix-cortex-a53-843419". In this case, the linker may convert an ADRP instruction appearing at an offset of 0xff8 or 0xffc modulo 4KiB into an ADR instruction, but will leave the original R_AARCH64_ADR_GOT_PAGE relocation in place. (This is not a bug in the linker, given that there is no other relocation type that it could reasonably convert it into) In this scenario, the following code is being generated by the toolchain: # Load to set the stack canary 2ffc: 10028020 adr x0, 8000 <mErrorString+0x1bc> 3008: f940d400 ldr x0, [x0, #424] # Load to check the stack canary 30cc: b0000020 adrp x0, 8000 <mErrorString+0x1bc> 30d0: f940d400 ldr x0, [x0, #424] GenFw rewrote that to: # Load to set the stack canary 2ffc: 10000480 adr x0, 0x308c 3008: 912ec000 add x0, x0, #0xbb0 # Load to check the stack canary 30cc: f0000460 adrp x0, 0x92000 30d0: 912ec000 add x0, x0, #0xbb0 Note that we're now setting the stack canary from the wrong address, resulting in an erroneous stack fault. After this fix, the offset will be calculated correctly for an ADR and the stack canary is set correctly. Note that there is a corner case where this may cause the conversion to fail: if the original GOT entry is just within -/+ 1 MiB of the reference, but the actual variable it refers to is not, the resulting offset cannot be represented by the immediate offset field in a ADR instruction. Given that this issue only affects PIE executables, which are rare and usually tiny, this is unlikely to cause problems in practice. Ref: https://edk2.groups.io/g/devel/topic/102202314 [ardb: expand commit log, add reference] Signed-off-by: Jake Garver <jake@nvidia.com> Reviewed-by: Rebecca Cran <rebecca@bsdio.com>

When converting ELF to PE/COFF for the AArch64 target, we may encounter an R_AARCH64_ADR_GOT_PAGE relocation that refers to an ADR instruction instead of an ADRP instruction. This can happen when the toolchain is working around Cortex-A53 erratum #843419. If that's the case, be sure to calculate the offset appropriately. This resolves an issue experienced when building a StandaloneMm image (which is built with -fpie) with stack protection enabled on GCC compiled with "--enable-fix-cortex-a53-843419". In this case, the linker may convert an ADRP instruction appearing at an offset of 0xff8 or 0xffc modulo 4KiB into an ADR instruction, but will leave the original R_AARCH64_ADR_GOT_PAGE relocation in place. (This is not a bug in the linker, given that there is no other relocation type that it could reasonably convert it into) In this scenario, the following code is being generated by the toolchain: # Load to set the stack canary 2ffc: 10028020 adr x0, 8000 <mErrorString+0x1bc> 3008: f940d400 ldr x0, [x0, tianocore#424] # Load to check the stack canary 30cc: b0000020 adrp x0, 8000 <mErrorString+0x1bc> 30d0: f940d400 ldr x0, [x0, tianocore#424] GenFw rewrote that to: # Load to set the stack canary 2ffc: 10000480 adr x0, 0x308c 3008: 912ec000 add x0, x0, #0xbb0 # Load to check the stack canary 30cc: f0000460 adrp x0, 0x92000 30d0: 912ec000 add x0, x0, #0xbb0 Note that we're now setting the stack canary from the wrong address, resulting in an erroneous stack fault. After this fix, the offset will be calculated correctly for an ADR and the stack canary is set correctly. Note that there is a corner case where this may cause the conversion to fail: if the original GOT entry is just within -/+ 1 MiB of the reference, but the actual variable it refers to is not, the resulting offset cannot be represented by the immediate offset field in a ADR instruction. Given that this issue only affects PIE executables, which are rare and usually tiny, this is unlikely to cause problems in practice. Ref: https://edk2.groups.io/g/devel/topic/102202314 [ardb: expand commit log, add reference] Change-Id: I347c27d9a4b3b576b216b9cad776e767bb160038 Signed-off-by: Jake Garver <jake@nvidia.com> Reviewed-by: Rebecca Cran <rebecca@bsdio.com> References: tianocore#67 Reviewed-on: https://git-master.nvidia.com/r/c/3rdparty/edk2/+/3041200 Reviewed-by: svcacv <svcacv@nvidia.com> Reviewed-by: svc-mobile-coverity <svc-mobile-coverity@nvidia.com> Reviewed-by: svc-sw-mobile-l4t <svc-sw-mobile-l4t@nvidia.com> GVS: Gerrit_Virtual_Submit <buildbot_gerritrpt@nvidia.com>

When converting ELF to PE/COFF for the AArch64 target, we may encounter an R_AARCH64_ADR_GOT_PAGE relocation that refers to an ADR instruction instead of an ADRP instruction. This can happen when the toolchain is working around Cortex-A53 erratum #843419. If that's the case, be sure to calculate the offset appropriately. This resolves an issue experienced when building a StandaloneMm image (which is built with -fpie) with stack protection enabled on GCC compiled with "--enable-fix-cortex-a53-843419". In this case, the linker may convert an ADRP instruction appearing at an offset of 0xff8 or 0xffc modulo 4KiB into an ADR instruction, but will leave the original R_AARCH64_ADR_GOT_PAGE relocation in place. (This is not a bug in the linker, given that there is no other relocation type that it could reasonably convert it into) In this scenario, the following code is being generated by the toolchain: # Load to set the stack canary 2ffc: 10028020 adr x0, 8000 <mErrorString+0x1bc> 3008: f940d400 ldr x0, [x0, tianocore#424] # Load to check the stack canary 30cc: b0000020 adrp x0, 8000 <mErrorString+0x1bc> 30d0: f940d400 ldr x0, [x0, tianocore#424] GenFw rewrote that to: # Load to set the stack canary 2ffc: 10000480 adr x0, 0x308c 3008: 912ec000 add x0, x0, #0xbb0 # Load to check the stack canary 30cc: f0000460 adrp x0, 0x92000 30d0: 912ec000 add x0, x0, #0xbb0 Note that we're now setting the stack canary from the wrong address, resulting in an erroneous stack fault. After this fix, the offset will be calculated correctly for an ADR and the stack canary is set correctly. Note that there is a corner case where this may cause the conversion to fail: if the original GOT entry is just within -/+ 1 MiB of the reference, but the actual variable it refers to is not, the resulting offset cannot be represented by the immediate offset field in a ADR instruction. Given that this issue only affects PIE executables, which are rare and usually tiny, this is unlikely to cause problems in practice. Ref: https://edk2.groups.io/g/devel/topic/102202314 [ardb: expand commit log, add reference] Signed-off-by: Jake Garver <jake@nvidia.com> Reviewed-by: Rebecca Cran <rebecca@bsdio.com>

Ard Biesheuvel added 14 commits March 5, 2020 14:21

ardbiesheuvel added the push Auto push patch series in PR if all checks pass label Mar 5, 2020

mergify bot merged commit ced7733 into tianocore:master Mar 5, 2020

ardbiesheuvel deleted the ovmf-loadimage-startimage-v3 branch March 26, 2020 08:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ovmf loadimage startimage v3 #424

Ovmf loadimage startimage v3 #424

ardbiesheuvel commented Mar 5, 2020

Ovmf loadimage startimage v3 #424

Ovmf loadimage startimage v3 #424

Conversation

ardbiesheuvel commented Mar 5, 2020