From 3f73e79de83ecc78b9a9c823b8118ab1fba63b0a Mon Sep 17 00:00:00 2001 From: Tom Rini Date: Fri, 19 Nov 2021 16:33:04 -0500 Subject: efi: Call bootm_disable_interrupts earlier in efi_exit_boot_services If we look at the path that bootm/booti take when preparing to boot the OS, we see that as part of (or prior to calling do_bootm_states, explicitly) the process, bootm_disable_interrupts() is called prior to announce_and_cleanup() which is where udc_disconnect() / board_quiesce_devices() / dm_remove_devices_flags() are called from. In the EFI path, these are called afterwards. In efi_exit_boot_services() however we have been calling bootm_disable_interrupts() after the above functions, as part of ensuring that we disable interrupts as required by the spec. However, bootm_disable_interrupts() is also where we go and call usb_stop(). While this has been fine before, on the TI J721E platform this leads us to an exception. This exception seems likely to be the case that we're trying to stop devices that we have already disabled clocks for. The most direct way to handle this particular problem is to make EFI behave like the do_bootm_states() process and ensure we call bootm_disable_interrupts() prior to ending up in usb_stop(). Cc: Ilias Apalodimas Cc: Heinrich Schuchardt Cc: Simon Glass Suggested-by: Ilias Apalodimas Signed-off-by: Tom Rini Reviewed-by: Ilias Apalodimas --- lib/efi_loader/efi_boottime.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 6fdd0ef77a..8492b732f3 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -2167,6 +2167,7 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, } if (!efi_st_keep_devices) { + bootm_disable_interrupts(); if (IS_ENABLED(CONFIG_USB_DEVICE)) udc_disconnect(); board_quiesce_devices(); @@ -2179,9 +2180,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, /* Fix up caches for EFI payloads if necessary */ efi_exit_caches(); - /* This stops all lingering devices */ - bootm_disable_interrupts(); - /* Disable boot time services */ systab.con_in_handle = NULL; systab.con_in = NULL; -- cgit v1.2.3 From cd9a26bfe56344822c75edbc771526099efe63e6 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 20 Nov 2021 13:56:02 +0100 Subject: efi_loader: efi_disk_register() should not fail Our algorithm for creating USB device paths may lead to duplicate device paths which result in efi_disk_register() failing. Instead we should just skip devices that cannot be registered as EFI block devices. Fix a memory leak in efi_disk_add_dev() caused by the duplicate device path. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_disk.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index ef8b5c88ff..45127d1768 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -424,7 +424,7 @@ static efi_status_t efi_disk_add_dev( &efi_block_io_guid, &diskobj->ops, guid, NULL, NULL)); if (ret != EFI_SUCCESS) - return ret; + goto error; /* * On partitions or whole disks without partitions install the @@ -573,7 +573,7 @@ efi_status_t efi_disk_register(void) if (ret) { log_err("ERROR: failure to add disk device %s, r = %lu\n", dev->name, ret & ~EFI_ERROR_MASK); - return ret; + continue; } disks++; -- cgit v1.2.3 From 9d1564dabc5897ebaf2fad842d800473790479a2 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 20 Nov 2021 11:53:12 +0100 Subject: efi_loader: segfault in efi_clear_os_indications() If we call efi_clear_os_indications() before initializing the memory store for UEFI variables a NULL pointer dereference occurs. The error was observed on the sandbox with: usb start host bind 0 sandbox.img load host 0:1 $kernel_addr_r helloworld.efi bootefi $kernel_addr_r Here efi_resister_disk() failed due to an error in the BTRFS implementation. Move the logic to clear EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED to the rest of the capsule code. If CONFIG_EFI_IGNORE_OSINDICATIONS=y, we should still clear the flag. If OsIndications does not exist, we should not create it as it is owned by the operating system. Fixes: 149108a3eb59 ("efi_loader: clear OsIndications") Signed-off-by: Heinrich Schuchardt Acked-by: Ilias Apalodimas --- lib/efi_loader/efi_capsule.c | 45 +++++++++++++++++++++++++++++--------------- lib/efi_loader/efi_setup.c | 36 +---------------------------------- 2 files changed, 31 insertions(+), 50 deletions(-) (limited to 'lib') diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index 502bcfca6e..8301eed631 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -1037,30 +1037,45 @@ efi_status_t __weak efi_load_capsule_drivers(void) } /** - * check_run_capsules - Check whether capsule update should run + * check_run_capsules() - check whether capsule update should run * * The spec says OsIndications must be set in order to run the capsule update * on-disk. Since U-Boot doesn't support runtime SetVariable, allow capsules to * run explicitly if CONFIG_EFI_IGNORE_OSINDICATIONS is selected + * + * Return: EFI_SUCCESS if update to run, EFI_NOT_FOUND otherwise */ -static bool check_run_capsules(void) +static efi_status_t check_run_capsules(void) { u64 os_indications; efi_uintn_t size; - efi_status_t ret; - - if (IS_ENABLED(CONFIG_EFI_IGNORE_OSINDICATIONS)) - return true; + efi_status_t r; size = sizeof(os_indications); - ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid, - NULL, &size, &os_indications, NULL); - if (ret == EFI_SUCCESS && - (os_indications - & EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED)) - return true; - - return false; + r = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid, + NULL, &size, &os_indications, NULL); + if (r != EFI_SUCCESS || size != sizeof(os_indications)) + return EFI_NOT_FOUND; + + if (os_indications & + EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED) { + os_indications &= + ~EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED; + r = efi_set_variable_int(L"OsIndications", + &efi_global_variable_guid, + EFI_VARIABLE_NON_VOLATILE | + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, + sizeof(os_indications), + &os_indications, false); + if (r != EFI_SUCCESS) + log_err("Setting %ls failed\n", L"OsIndications"); + return EFI_SUCCESS; + } else if (IS_ENABLED(CONFIG_EFI_IGNORE_OSINDICATIONS)) { + return EFI_SUCCESS; + } else { + return EFI_NOT_FOUND; + } } /** @@ -1078,7 +1093,7 @@ efi_status_t efi_launch_capsules(void) unsigned int nfiles, index, i; efi_status_t ret; - if (!check_run_capsules()) + if (check_run_capsules() != EFI_SUCCESS) return EFI_SUCCESS; index = get_last_capsule(); diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c index a2338d74af..1aba71cd96 100644 --- a/lib/efi_loader/efi_setup.c +++ b/lib/efi_loader/efi_setup.c @@ -175,36 +175,6 @@ static efi_status_t efi_init_os_indications(void) } -/** - * efi_clear_os_indications() - clear OsIndications - * - * Clear EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED - */ -static efi_status_t efi_clear_os_indications(void) -{ - efi_uintn_t size; - u64 os_indications; - efi_status_t ret; - - size = sizeof(os_indications); - ret = efi_get_variable_int(L"OsIndications", &efi_global_variable_guid, - NULL, &size, &os_indications, NULL); - if (ret != EFI_SUCCESS) - os_indications = 0; - else - os_indications &= - ~EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED; - ret = efi_set_variable_int(L"OsIndications", &efi_global_variable_guid, - EFI_VARIABLE_NON_VOLATILE | - EFI_VARIABLE_BOOTSERVICE_ACCESS | - EFI_VARIABLE_RUNTIME_ACCESS, - sizeof(os_indications), &os_indications, - false); - if (ret != EFI_SUCCESS) - log_err("Setting %ls failed\n", L"OsIndications"); - return ret; -} - /** * efi_init_obj_list() - Initialize and populate EFI object list * @@ -212,7 +182,7 @@ static efi_status_t efi_clear_os_indications(void) */ efi_status_t efi_init_obj_list(void) { - efi_status_t r, ret = EFI_SUCCESS; + efi_status_t ret = EFI_SUCCESS; /* Initialize once only */ if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED) @@ -331,11 +301,7 @@ efi_status_t efi_init_obj_list(void) if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK) && !IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK_EARLY)) ret = efi_launch_capsules(); - out: - r = efi_clear_os_indications(); - if (ret == EFI_SUCCESS) - ret = r; efi_obj_list_initialized = ret; return ret; } -- cgit v1.2.3 From 9abd2ca96ea0e8e68a9ffb5b41d1107ea73ddc0f Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 25 Nov 2021 18:55:09 +0100 Subject: efi_selftest: simplify endian conversion for FDT test UEFI code is always little-endian. Remove a superfluous test. Remove a superfluous type conversion. Signed-off-by: Heinrich Schuchardt --- lib/efi_selftest/efi_selftest_fdt.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'lib') diff --git a/lib/efi_selftest/efi_selftest_fdt.c b/lib/efi_selftest/efi_selftest_fdt.c index eae98208f6..412ba286ea 100644 --- a/lib/efi_selftest/efi_selftest_fdt.c +++ b/lib/efi_selftest/efi_selftest_fdt.c @@ -23,23 +23,24 @@ static const char *fdt; static const efi_guid_t fdt_guid = EFI_FDT_GUID; static const efi_guid_t acpi_guid = EFI_ACPI_TABLE_GUID; -/* - * Convert FDT value to host endianness. +/** + * f2h() - convert FDT value to host endianness. * - * @val FDT value - * @return converted value + * UEFI code is always low endian. The FDT is big endian. + * + * @val: FDT value + * Return: converted value */ static uint32_t f2h(fdt32_t val) { char *buf = (char *)&val; char i; -#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ /* Swap the bytes */ i = buf[0]; buf[0] = buf[3]; buf[3] = i; i = buf[1]; buf[1] = buf[2]; buf[2] = i; -#endif - return *(uint32_t *)buf; + + return val; } /** -- cgit v1.2.3