From f03a879d67261b587a88e8e475596c1bbe7e6111 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 22 Aug 2020 08:29:53 +0200 Subject: efi_loader: ResetSystem() should not hang If ResetSystem() is not implemented at runtime, it should return instead of hanging in an endless loop. This allows the operating system to reset the system by other means as Linux does. It also matches what EDK II suggests in comments for functions ResetShutdown() and ResetWarm() in OvmfPkg/Library/ResetSystemLib/ResetSystemLib.c. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_runtime.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index 78fd8014d90..dea2b4e5eea 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -365,7 +365,9 @@ out: * efi_reset_system() - reset system * * This function implements the ResetSystem() runtime service after - * SetVirtualAddressMap() is called. It only executes an endless loop. + * SetVirtualAddressMap() is called. As this placeholder cannot reset the + * system it simply return to the caller. + * * Boards may override the helpers below to implement reset functionality. * * See the Unified Extensible Firmware Interface (UEFI) specification for @@ -381,8 +383,7 @@ void __weak __efi_runtime EFIAPI efi_reset_system( efi_status_t reset_status, unsigned long data_size, void *reset_data) { - /* Nothing we can do */ - while (1) { } + return; } /** -- cgit v1.2.3 From c06867d7f8f64606a16ce45e8ac07fdc3ace4f13 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 20 Aug 2020 12:16:54 +0200 Subject: efi_selftest: add a test for ResetSystem() The unit test will reset the system by calling the ResetSystem() runtime service before or after ExitBootServices() according to the users choice by setting environment variable efi_selftest to: * 'reset system' or * 'reset system runtime'. Signed-off-by: Heinrich Schuchardt --- lib/efi_selftest/Makefile | 1 + lib/efi_selftest/efi_selftest_reset.c | 58 +++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 lib/efi_selftest/efi_selftest_reset.c (limited to 'lib') diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile index 45ce6859b86..85fe8e1216a 100644 --- a/lib/efi_selftest/Makefile +++ b/lib/efi_selftest/Makefile @@ -31,6 +31,7 @@ efi_selftest_mem.o \ efi_selftest_memory.o \ efi_selftest_open_protocol.o \ efi_selftest_register_notify.o \ +efi_selftest_reset.o \ efi_selftest_set_virtual_address_map.o \ efi_selftest_textinput.o \ efi_selftest_textinputex.o \ diff --git a/lib/efi_selftest/efi_selftest_reset.c b/lib/efi_selftest/efi_selftest_reset.c new file mode 100644 index 00000000000..8b6ac24cb15 --- /dev/null +++ b/lib/efi_selftest/efi_selftest_reset.c @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * efi_selftest_reset + * + * Copyright (c) 2020 Heinrich Schuchardt + * + * This test checks the following service at boot time or runtime: + * ResetSystem() + */ + +#include + +static struct efi_runtime_services *runtime; + +/* + * 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) +{ + runtime = systable->runtime; + return EFI_ST_SUCCESS; +} + +/* + * Execute unit test. + * + * @return: EFI_ST_SUCCESS for success + */ +static int execute(void) +{ + u16 reset_data[] = L"Reset by selftest"; + + runtime->reset_system(EFI_RESET_COLD, EFI_SUCCESS, + sizeof(reset_data), reset_data); + efi_st_error("Reset failed.\n"); + return EFI_ST_FAILURE; +} + +EFI_UNIT_TEST(reset) = { + .name = "reset system", + .phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT, + .setup = setup, + .execute = execute, + .on_request = true, +}; + +EFI_UNIT_TEST(resetrt) = { + .name = "reset system runtime", + .phase = EFI_SETUP_BEFORE_BOOTTIME_EXIT, + .setup = setup, + .execute = execute, + .on_request = true, +}; -- cgit v1.2.3 From fa63753f86ccf912d2553934ee6aec787030fa8a Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 22 Aug 2020 09:14:56 +0200 Subject: efi_selftest: substitute ResetSystem() by do_reset() If ResetSystem() is not implemented at runtime, call do_reset() after test completion. Signed-off-by: Heinrich Schuchardt --- lib/efi_selftest/efi_selftest.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/efi_selftest/efi_selftest.c b/lib/efi_selftest/efi_selftest.c index 5b01610eca1..6eec8ae2a7c 100644 --- a/lib/efi_selftest/efi_selftest.c +++ b/lib/efi_selftest/efi_selftest.c @@ -5,6 +5,7 @@ * Copyright (c) 2017 Heinrich Schuchardt */ +#include #include #include @@ -309,8 +310,13 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle, /* Reset system */ efi_st_printf("Preparing for reset. Press any key...\n"); efi_st_get_key(); - runtime->reset_system(EFI_RESET_WARM, EFI_NOT_READY, - sizeof(reset_message), reset_message); + + if (IS_ENABLED(CONFIG_EFI_HAVE_RUNTIME_RESET)) + runtime->reset_system(EFI_RESET_WARM, EFI_NOT_READY, + sizeof(reset_message), reset_message); + else + do_reset(NULL, 0, 0, NULL); + efi_st_printf("\n"); efi_st_error("Reset failed\n"); -- cgit v1.2.3 From 5cad4a30932a31f1646510d35af7e9e36f71708a Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sun, 23 Aug 2020 10:49:46 +0200 Subject: efi_loader: efi_dp_check_length() We need to check that device paths provided via UEFI variables are not malformed. Provide function efi_dp_check_length() to check if a device path has an end node within a given number of bytes. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_device_path.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) (limited to 'lib') diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 7ae14f34239..8a5c13c4241 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -1127,3 +1127,36 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr, return EFI_SUCCESS; } + +/** + * efi_dp_check_length() - check length of a device path + * + * @dp: pointer to device path + * @maxlen: maximum length of the device path + * Return: + * * length of the device path if it is less or equal @maxlen + * * -1 if the device path is longer then @maxlen + * * -1 if a device path node has a length of less than 4 + * * -EINVAL if maxlen exceeds SSIZE_MAX + */ +ssize_t efi_dp_check_length(const struct efi_device_path *dp, + const size_t maxlen) +{ + ssize_t ret = 0; + u16 len; + + if (maxlen > SSIZE_MAX) + return -EINVAL; + for (;;) { + len = dp->length; + if (len < 4) + return -1; + ret += len; + if (ret > maxlen) + return -1; + if (dp->type == DEVICE_PATH_TYPE_END && + dp->sub_type == DEVICE_PATH_SUB_TYPE_END) + return ret; + dp = (const struct efi_device_path *)((const u8 *)dp + len); + } +} -- cgit v1.2.3 From 15d8f008dc577bbef6ad98494c28553558941420 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sun, 23 Aug 2020 10:59:17 +0200 Subject: efi_loader: validate device path length in boot manager Bootxxxx variables are provided by the user and therefore cannot be trusted. We have to validate them before usage. A device path provided by a Bootxxxx variable must have an end node within the indicated device path length. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_bootmgr.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 1e06e609639..61dc72a23da 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -105,10 +105,8 @@ efi_status_t efi_deserialize_load_option(struct efi_load_option *lo, u8 *data, if (*size < len) return EFI_INVALID_PARAMETER; lo->file_path = (struct efi_device_path *)data; - /* - * TODO: validate device path. There should be an end node within - * the indicated file_path_length. - */ + if (efi_dp_check_length(lo->file_path, len) < 0) + return EFI_INVALID_PARAMETER; data += len; *size -= len; -- cgit v1.2.3