[U-Boot] [PATCH v3 0/2] efi_loader: correctly call images

This patch series fixes various problems with the StartImage boot service. It further provides unit tests.
v3 Supply two separate unit tests covering that the application either returns by calling the Exit service or with a simple return. v2 Do not build test on x68_64 due to a problem with the build system for EFI images.
Heinrich Schuchardt (2): efi_loader: correctly call images efi_selftest: test start image
arch/arm/lib/Makefile | 1 + lib/efi_loader/efi_boottime.c | 21 ++- lib/efi_selftest/.gitignore | 2 + lib/efi_selftest/Makefile | 35 +++++ lib/efi_selftest/efi_selftest_miniapp_exit.c | 37 ++++++ lib/efi_selftest/efi_selftest_miniapp_return.c | 32 +++++ lib/efi_selftest/efi_selftest_startimage_exit.c | 149 ++++++++++++++++++++++ lib/efi_selftest/efi_selftest_startimage_return.c | 149 ++++++++++++++++++++++ 8 files changed, 419 insertions(+), 7 deletions(-) create mode 100644 lib/efi_selftest/.gitignore create mode 100644 lib/efi_selftest/efi_selftest_miniapp_exit.c create mode 100644 lib/efi_selftest/efi_selftest_miniapp_return.c create mode 100644 lib/efi_selftest/efi_selftest_startimage_exit.c create mode 100644 lib/efi_selftest/efi_selftest_startimage_return.c

Avoid a failed assertion when an EFI app calls an EFI app.
Avoid that the indent level increases when calling 'bootefi hello' repeatedly.
Avoid negative indent level when an EFI app calls an EFI app that calls an EFI app (e.g. iPXE loads grub which starts the kernel).
Return the status code of a loaded image that returns without calling the Exit boot service.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 Provide more comments. --- lib/efi_loader/efi_boottime.c | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 2c5499e0c8..49cd69203b 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1537,6 +1537,7 @@ static efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, asmlinkage ulong (*entry)(efi_handle_t image_handle, struct efi_system_table *st); struct efi_loaded_image *info = image_handle; + efi_status_t ret;
EFI_ENTRY("%p, %p, %p", image_handle, exit_data_size, exit_data); entry = info->reserved; @@ -1545,18 +1546,37 @@ static efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
/* call the image! */ if (setjmp(&info->exit_jmp)) { - /* We returned from the child image */ + /* + * We called the entry point of the child image with EFI_CALL + * in the lines below. The child image called the Exit() boot + * service efi_exit() which executed the long jump that brought + * us to the current line. This implies that the second half + * of the EFI_CALL macro has not been executed. + */ +#ifdef CONFIG_ARM + /* + * efi_exit() called efi_restore_gd(). We have to undo this + * otherwise __efi_entry_check() will put the wrong value into + * app_gd. + */ + gd = app_gd; +#endif + /* + * To get ready to call EFI_EXIT below we have to execute the + * missed out steps of EFI_CALL. + */ + assert(__efi_entry_check()); + debug("%sEFI: %lu returned by started image\n", + __efi_nesting_dec(), + (unsigned long)((uintptr_t)info->exit_status & + ~EFI_ERROR_MASK)); return EFI_EXIT(info->exit_status); }
- __efi_nesting_dec(); - __efi_exit_check(); - entry(image_handle, &systab); - __efi_entry_check(); - __efi_nesting_inc(); + ret = EFI_CALL(entry(image_handle, &systab));
/* Should usually never get here */ - return EFI_EXIT(EFI_SUCCESS); + return EFI_EXIT(ret); }
/* @@ -1593,7 +1613,7 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, exit_data_size, exit_data);
/* Make sure entry/exit counts for EFI world cross-overs match */ - __efi_exit_check(); + EFI_EXIT(exit_status);
/* * But longjmp out with the U-Boot gd, not the application's, as

This pair of tests checks the StartImage boot service.
Each test loads an EFI application into memory and starts it. One returns by calling the Exit boot service. The other returns directly.
The tests are not built on x86_64 because the relocation code for the efi binary cannot be created.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3 Supply two separate unit tests covering that the application either returns by calling the Exit service or with a simple return. v2 Do not build test on x86_64. --- arch/arm/lib/Makefile | 1 + lib/efi_selftest/.gitignore | 2 + lib/efi_selftest/Makefile | 35 +++++ lib/efi_selftest/efi_selftest_miniapp_exit.c | 37 ++++++ lib/efi_selftest/efi_selftest_miniapp_return.c | 32 +++++ lib/efi_selftest/efi_selftest_startimage_exit.c | 149 ++++++++++++++++++++++ lib/efi_selftest/efi_selftest_startimage_return.c | 149 ++++++++++++++++++++++ 7 files changed, 405 insertions(+) create mode 100644 lib/efi_selftest/.gitignore create mode 100644 lib/efi_selftest/efi_selftest_miniapp_exit.c create mode 100644 lib/efi_selftest/efi_selftest_miniapp_return.c create mode 100644 lib/efi_selftest/efi_selftest_startimage_exit.c create mode 100644 lib/efi_selftest/efi_selftest_startimage_return.c
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index abffa10c85..876024fc15 100644 --- a/arch/arm/lib/Makefile +++ b/arch/arm/lib/Makefile @@ -112,4 +112,5 @@ CFLAGS_$(EFI_RELOC) := $(CFLAGS_EFI) CFLAGS_REMOVE_$(EFI_RELOC) := $(CFLAGS_NON_EFI)
extra-$(CONFIG_CMD_BOOTEFI_HELLO_COMPILE) += $(EFI_CRT0) $(EFI_RELOC) +extra-$(CONFIG_CMD_BOOTEFI_SELFTEST) += $(EFI_CRT0) $(EFI_RELOC) extra-$(CONFIG_EFI) += $(EFI_CRT0) $(EFI_RELOC) diff --git a/lib/efi_selftest/.gitignore b/lib/efi_selftest/.gitignore new file mode 100644 index 0000000000..c527e464e5 --- /dev/null +++ b/lib/efi_selftest/.gitignore @@ -0,0 +1,2 @@ +efi_miniapp_file_image.h +*.efi diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile index 20f614d6ba..90246f7827 100644 --- a/lib/efi_selftest/Makefile +++ b/lib/efi_selftest/Makefile @@ -7,6 +7,9 @@ # This file only gets included with CONFIG_EFI_LOADER set, so all # object inclusion implicitly depends on it
+CFLAGS_efi_selftest_miniapp.o := $(CFLAGS_EFI) -Os -ffreestanding +CFLAGS_REMOVE_efi_selftest_miniapp.o := $(CFLAGS_NON_EFI) -Os + obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += \ efi_selftest.o \ efi_selftest_controllers.o \ @@ -25,3 +28,35 @@ efi_selftest_watchdog.o ifeq ($(CONFIG_BLK)$(CONFIG_PARTITIONS),yy) obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += efi_selftest_block_device.o endif + +# TODO: As of v2018.01 the relocation code for the EFI application cannot +# be built on x86_64. +ifeq ($(CONFIG_X86_64),) + +ifneq ($(CONFIG_CMD_BOOTEFI_SELFTEST),) + +obj-y += \ +efi_selftest_startimage_exit.o \ +efi_selftest_startimage_return.o + +targets += \ +efi_miniapp_file_image_exit.h \ +efi_miniapp_file_image_return.h \ +efi_selftest_miniapp_exit.efi \ +efi_selftest_miniapp_return.efi + +$(obj)/efi_miniapp_file_image_exit.h: $(obj)/efi_selftest_miniapp_exit.efi + $(obj)/../../tools/file2include $(obj)/efi_selftest_miniapp_exit.efi > \ + $(obj)/efi_miniapp_file_image_exit.h + +$(obj)/efi_miniapp_file_image_return.h: $(obj)/efi_selftest_miniapp_return.efi + $(obj)/../../tools/file2include $(obj)/efi_selftest_miniapp_return.efi > \ + $(obj)/efi_miniapp_file_image_return.h + +$(obj)/efi_selftest_startimage_exit.o: $(obj)/efi_miniapp_file_image_exit.h + +$(obj)/efi_selftest_startimage_return.o: $(obj)/efi_miniapp_file_image_return.h + +endif + +endif diff --git a/lib/efi_selftest/efi_selftest_miniapp_exit.c b/lib/efi_selftest/efi_selftest_miniapp_exit.c new file mode 100644 index 0000000000..590c948a1c --- /dev/null +++ b/lib/efi_selftest/efi_selftest_miniapp_exit.c @@ -0,0 +1,37 @@ +/* + * efi_selftest_miniapp_exit + * + * Copyright (c) 2018 Heinrich Schuchardt + * + * SPDX-License-Identifier: GPL-2.0+ + * + * This EFI application is run by the StartImage selftest. + * It uses the Exit boot service to return. + */ + +#include <common.h> +#include <efi_api.h> + +/* + * Entry point of the EFI application. + * + * @handle handle of the loaded image + * @systable system table + * @return status code + */ +efi_status_t EFIAPI efi_main(efi_handle_t handle, + struct efi_system_table *systable) +{ + struct efi_simple_text_output_protocol *con_out = systable->con_out; + + con_out->output_string(con_out, L"EFI application calling Exit"); + + /* The return value is checked by the calling test */ + systable->boottime->exit(handle, EFI_UNSUPPORTED, 0, NULL); + + /* + * This statement should not be reached. + * To enable testing use a different return value. + */ + return EFI_SUCCESS; +} diff --git a/lib/efi_selftest/efi_selftest_miniapp_return.c b/lib/efi_selftest/efi_selftest_miniapp_return.c new file mode 100644 index 0000000000..e5d3979b03 --- /dev/null +++ b/lib/efi_selftest/efi_selftest_miniapp_return.c @@ -0,0 +1,32 @@ +/* + * efi_selftest_miniapp_return + * + * Copyright (c) 2018 Heinrich Schuchardt + * + * SPDX-License-Identifier: GPL-2.0+ + * + * This EFI application is run by the StartImage selftest. + * It returns directly without calling the Exit boot service. + */ + +#include <common.h> +#include <efi_api.h> + +/* + * Entry point of the EFI application. + * + * @handle handle of the loaded image + * @systable system table + * @return status code + */ +efi_status_t EFIAPI efi_main(efi_handle_t handle, + struct efi_system_table *systable) +{ + struct efi_simple_text_output_protocol *con_out = systable->con_out; + + con_out->output_string(con_out, + L"EFI application returning w/o calling Exit"); + + /* The return value is checked by the calling test */ + return EFI_INCOMPATIBLE_VERSION; +} diff --git a/lib/efi_selftest/efi_selftest_startimage_exit.c b/lib/efi_selftest/efi_selftest_startimage_exit.c new file mode 100644 index 0000000000..0809690e97 --- /dev/null +++ b/lib/efi_selftest/efi_selftest_startimage_exit.c @@ -0,0 +1,149 @@ +/* + * efi_selftest_start_image + * + * Copyright (c) 2018 Heinrich Schuchardt xypron.glpk@gmx.de + * + * SPDX-License-Identifier: GPL-2.0+ + * + * This test checks the StartImage boot service. + * The efi_selftest_miniapp_exit.efi application is loaded into memory + * and started. + */ + +#include <efi_selftest.h> +/* Include containing the miniapp.efi application */ +#include "efi_miniapp_file_image_exit.h" + +/* Block size of compressed disk image */ +#define COMPRESSED_DISK_IMAGE_BLOCK_SIZE 8 + +/* Binary logarithm of the block size */ +#define LB_BLOCK_SIZE 9 + +static efi_handle_t image_handle; +static struct efi_boot_services *boottime; + +/* One 8 byte block of the compressed disk image */ +struct line { + size_t addr; + char *line; +}; + +/* Compressed file image */ +struct compressed_file_image { + size_t length; + struct line lines[]; +}; + +static struct compressed_file_image img = EFI_ST_DISK_IMG; + +/* Decompressed file image */ +static u8 *image; + +/* + * Decompress the disk image. + * + * @image decompressed disk image + * @return status code + */ +static efi_status_t decompress(u8 **image) +{ + u8 *buf; + size_t i; + size_t addr; + size_t len; + efi_status_t ret; + + ret = boottime->allocate_pool(EFI_LOADER_DATA, img.length, + (void **)&buf); + if (ret != EFI_SUCCESS) { + efi_st_error("Out of memory\n"); + return ret; + } + boottime->set_mem(buf, img.length, 0); + + for (i = 0; ; ++i) { + if (!img.lines[i].line) + break; + addr = img.lines[i].addr; + len = COMPRESSED_DISK_IMAGE_BLOCK_SIZE; + if (addr + len > img.length) + len = img.length - addr; + boottime->copy_mem(buf + addr, img.lines[i].line, len); + } + *image = buf; + return ret; +} + +/* + * Setup unit test. + * + * @handle: handle of the loaded image + * @systable: system table + * @return: EFI_ST_SUCCESS for success + */ +static int setup(const efi_handle_t handle, + const struct efi_system_table *systable) +{ + image_handle = handle; + boottime = systable->boottime; + + /* Load the application image into memory */ + decompress(&image); + + return EFI_ST_SUCCESS; +} + +/* + * Tear down unit test. + * + * @return: EFI_ST_SUCCESS for success + */ +static int teardown(void) +{ + efi_status_t r = EFI_ST_SUCCESS; + + if (image) { + r = efi_free_pool(image); + if (r != EFI_SUCCESS) { + efi_st_error("Failed to free image\n"); + return EFI_ST_FAILURE; + } + } + return r; +} + +/* + * Execute unit test. + * + * Load and start the application image. + * + * @return: EFI_ST_SUCCESS for success + */ +static int execute(void) +{ + efi_status_t ret; + efi_handle_t handle; + + ret = boottime->load_image(false, image_handle, NULL, image, + img.length, &handle); + if (ret != EFI_SUCCESS) { + efi_st_error("Failed to load image\n"); + return EFI_ST_FAILURE; + } + ret = boottime->start_image(handle, NULL, NULL); + if (ret != EFI_UNSUPPORTED) { + efi_st_error("Wrong return value from application\n"); + return EFI_ST_FAILURE; + } + + return EFI_ST_SUCCESS; +} + +EFI_UNIT_TEST(startimage_exit) = { + .name = "start image exit", + .phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT, + .setup = setup, + .execute = execute, + .teardown = teardown, +}; diff --git a/lib/efi_selftest/efi_selftest_startimage_return.c b/lib/efi_selftest/efi_selftest_startimage_return.c new file mode 100644 index 0000000000..51fb853c79 --- /dev/null +++ b/lib/efi_selftest/efi_selftest_startimage_return.c @@ -0,0 +1,149 @@ +/* + * efi_selftest_start_image + * + * Copyright (c) 2018 Heinrich Schuchardt xypron.glpk@gmx.de + * + * SPDX-License-Identifier: GPL-2.0+ + * + * This test checks the StartImage boot service. + * The efi_selftest_miniapp_return.efi application is loaded into memory + * and started. + */ + +#include <efi_selftest.h> +/* Include containing the miniapp.efi application */ +#include "efi_miniapp_file_image_return.h" + +/* Block size of compressed disk image */ +#define COMPRESSED_DISK_IMAGE_BLOCK_SIZE 8 + +/* Binary logarithm of the block size */ +#define LB_BLOCK_SIZE 9 + +static efi_handle_t image_handle; +static struct efi_boot_services *boottime; + +/* One 8 byte block of the compressed disk image */ +struct line { + size_t addr; + char *line; +}; + +/* Compressed file image */ +struct compressed_file_image { + size_t length; + struct line lines[]; +}; + +static struct compressed_file_image img = EFI_ST_DISK_IMG; + +/* Decompressed file image */ +static u8 *image; + +/* + * Decompress the disk image. + * + * @image decompressed disk image + * @return status code + */ +static efi_status_t decompress(u8 **image) +{ + u8 *buf; + size_t i; + size_t addr; + size_t len; + efi_status_t ret; + + ret = boottime->allocate_pool(EFI_LOADER_DATA, img.length, + (void **)&buf); + if (ret != EFI_SUCCESS) { + efi_st_error("Out of memory\n"); + return ret; + } + boottime->set_mem(buf, img.length, 0); + + for (i = 0; ; ++i) { + if (!img.lines[i].line) + break; + addr = img.lines[i].addr; + len = COMPRESSED_DISK_IMAGE_BLOCK_SIZE; + if (addr + len > img.length) + len = img.length - addr; + boottime->copy_mem(buf + addr, img.lines[i].line, len); + } + *image = buf; + return ret; +} + +/* + * Setup unit test. + * + * @handle: handle of the loaded image + * @systable: system table + * @return: EFI_ST_SUCCESS for success + */ +static int setup(const efi_handle_t handle, + const struct efi_system_table *systable) +{ + image_handle = handle; + boottime = systable->boottime; + + /* Load the application image into memory */ + decompress(&image); + + return EFI_ST_SUCCESS; +} + +/* + * Tear down unit test. + * + * @return: EFI_ST_SUCCESS for success + */ +static int teardown(void) +{ + efi_status_t r = EFI_ST_SUCCESS; + + if (image) { + r = efi_free_pool(image); + if (r != EFI_SUCCESS) { + efi_st_error("Failed to free image\n"); + return EFI_ST_FAILURE; + } + } + return r; +} + +/* + * Execute unit test. + * + * Load and start the application image. + * + * @return: EFI_ST_SUCCESS for success + */ +static int execute(void) +{ + efi_status_t ret; + efi_handle_t handle; + + ret = boottime->load_image(false, image_handle, NULL, image, + img.length, &handle); + if (ret != EFI_SUCCESS) { + efi_st_error("Failed to load image\n"); + return EFI_ST_FAILURE; + } + ret = boottime->start_image(handle, NULL, NULL); + if (ret != EFI_INCOMPATIBLE_VERSION) { + efi_st_error("Wrong return value from application\n"); + return EFI_INCOMPATIBLE_VERSION; + } + + return EFI_ST_SUCCESS; +} + +EFI_UNIT_TEST(startimage) = { + .name = "start image return", + .phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT, + .setup = setup, + .execute = execute, + .teardown = teardown, +};
participants (1)
-
Heinrich Schuchardt