From 751e928d07ae6ddb2b770b3a4c7f74fd6f2a5a64 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 25 Apr 2019 07:30:41 +0200 Subject: efi_loader: parameter check CreateEventEx() CreateEvent() and CreateEventEx() should check that a notify function is provided for either of EVT_NOTIFY_SIGNAL or EVT_NOTIFY_WAIT. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_boottime.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 601b0a2cb8..a72cbe1559 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -618,7 +618,7 @@ efi_status_t efi_create_event(uint32_t type, efi_uintn_t notify_tpl, } if ((type & (EVT_NOTIFY_WAIT | EVT_NOTIFY_SIGNAL)) && - (is_valid_tpl(notify_tpl) != EFI_SUCCESS)) + (!notify_function || is_valid_tpl(notify_tpl) != EFI_SUCCESS)) return EFI_INVALID_PARAMETER; evt = calloc(1, sizeof(struct efi_event)); -- cgit v1.2.3 From e00b82db809725ead77ddfe602c9c8fad1f45ab7 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 25 Apr 2019 18:41:40 +0200 Subject: efi_loader: FreePages() must fail with pages = 0 The UEFI spec requires that freeing of pages fails if the number of pages to be freed is 'invalid'. Check that it is not zero. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 987cc6dc5f..776077cc35 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -452,7 +452,7 @@ efi_status_t efi_free_pages(uint64_t memory, efi_uintn_t pages) uint64_t r = 0; /* Sanity check */ - if (!memory || (memory & EFI_PAGE_MASK)) { + if (!memory || (memory & EFI_PAGE_MASK) || !pages) { printf("%s: illegal free 0x%llx, 0x%zx\n", __func__, memory, pages); return EFI_INVALID_PARAMETER; -- cgit v1.2.3 From d40e05ae95cfdf328ed6940fb48e110dc5da0c77 Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Wed, 24 Apr 2019 15:30:38 +0900 Subject: efi_loader: set OsIndicationsSupported at init UEFI variables should be installed using well-defined API. Currently we don't support much, but the value of OsIndicationsSupported will be updated once some features are added in the future. Signed-off-by: AKASHI Takahiro Add comments. Rename a variable. Reviewed-by: Heinrich Schuchardt --- cmd/bootefi.c | 4 ---- lib/efi_loader/efi_setup.c | 11 +++++++++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/cmd/bootefi.c b/cmd/bootefi.c index efaa548be4..b93d8c6a32 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -303,10 +303,6 @@ static efi_status_t do_bootefi_exec(efi_handle_t handle) if (ret != EFI_SUCCESS) return ret; - /* we don't support much: */ - env_set("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported", - "{ro,boot}(blob)0000000000000000"); - /* Call our payload! */ ret = EFI_CALL(efi_start_image(handle, NULL, NULL)); diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c index b32a7b3f93..87db51cbb7 100644 --- a/lib/efi_loader/efi_setup.c +++ b/lib/efi_loader/efi_setup.c @@ -79,6 +79,7 @@ out: */ efi_status_t efi_init_obj_list(void) { + u64 os_indications_supported = 0; /* None */ efi_status_t ret = EFI_SUCCESS; /* Initialize once only */ @@ -90,6 +91,16 @@ efi_status_t efi_init_obj_list(void) if (ret != EFI_SUCCESS) goto out; + /* Indicate supported features */ + ret = EFI_CALL(efi_set_variable(L"OsIndicationsSupported", + &efi_global_variable_guid, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, + sizeof(os_indications_supported), + &os_indications_supported)); + if (ret != EFI_SUCCESS) + goto out; + /* Initialize system table */ ret = efi_initialize_system_table(); if (ret != EFI_SUCCESS) -- cgit v1.2.3 From ffe2157447ce0101802e717147001fd59581e759 Mon Sep 17 00:00:00 2001 From: AKASHI Takahiro Date: Fri, 26 Apr 2019 09:44:18 +0900 Subject: cmd: efidebug: rework "boot dump" sub-command using GetNextVariableName() Currently in do_efi_boot_dump(), we directly read EFI variables from related environment variables. To accommodate alternative storage backends, we should switch to using the UEFI API instead. Signed-off-by: AKASHI Takahiro Reviewed-by: Heinrich Schuchardt --- cmd/efidebug.c | 79 ++++++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 55 insertions(+), 24 deletions(-) diff --git a/cmd/efidebug.c b/cmd/efidebug.c index a40c4f4be2..efe4ea873e 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -693,6 +693,17 @@ static void show_efi_boot_opt(int id) free(data); } +static int u16_tohex(u16 c) +{ + if (c >= '0' && c <= '9') + return c - '0'; + if (c >= 'A' && c <= 'F') + return c - 'A' + 10; + + /* not hexadecimal */ + return -1; +} + /** * show_efi_boot_dump() - dump all UEFI load options * @@ -709,38 +720,58 @@ static void show_efi_boot_opt(int id) static int do_efi_boot_dump(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { - char regex[256]; - char * const regexlist[] = {regex}; - char *variables = NULL, *boot, *value; - int len; - int id; + u16 *var_name16, *p; + efi_uintn_t buf_size, size; + efi_guid_t guid; + int id, i, digit; + efi_status_t ret; if (argc > 1) return CMD_RET_USAGE; - snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_Boot[0-9A-F]+"); - - /* TODO: use GetNextVariableName? */ - len = hexport_r(&env_htab, '\n', H_MATCH_REGEX | H_MATCH_KEY, - &variables, 0, 1, regexlist); - - if (!len) - return CMD_RET_SUCCESS; - - if (len < 0) + buf_size = 128; + var_name16 = malloc(buf_size); + if (!var_name16) return CMD_RET_FAILURE; - boot = variables; - while (*boot) { - value = strstr(boot, "Boot") + 4; - id = (int)simple_strtoul(value, NULL, 16); - show_efi_boot_opt(id); - boot = strchr(boot, '\n'); - if (!*boot) + var_name16[0] = 0; + for (;;) { + size = buf_size; + ret = EFI_CALL(efi_get_next_variable_name(&size, var_name16, + &guid)); + if (ret == EFI_NOT_FOUND) break; - boot++; + if (ret == EFI_BUFFER_TOO_SMALL) { + buf_size = size; + p = realloc(var_name16, buf_size); + if (!p) { + free(var_name16); + return CMD_RET_FAILURE; + } + var_name16 = p; + ret = EFI_CALL(efi_get_next_variable_name(&size, + var_name16, + &guid)); + } + if (ret != EFI_SUCCESS) { + free(var_name16); + return CMD_RET_FAILURE; + } + + if (memcmp(var_name16, L"Boot", 8)) + continue; + + for (id = 0, i = 0; i < 4; i++) { + digit = u16_tohex(var_name16[4 + i]); + if (digit < 0) + break; + id = (id << 4) + digit; + } + if (i == 4 && !var_name16[8]) + show_efi_boot_opt(id); } - free(variables); + + free(var_name16); return CMD_RET_SUCCESS; } -- cgit v1.2.3 From 39a1ff8ceab4b74164b2e19217206e7226aa9cd8 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Mon, 29 Apr 2019 13:51:45 +0200 Subject: efi_loader: optional data in load options are binary The field boot OptionalData in structure _EFI_LOAD_OPTIONS is for binary data. When we use `efidebug boot add` we should convert the 5th argument from UTF-8 to UTF-16 before putting it into the BootXXXX variable. When printing boot variables with `efidebug boot dump` we should support the OptionalData being arbitrary binary data. So let's dump the data as hexadecimal values. Here is an example session protocol: => efidebug boot add 00a1 label1 scsi 0:1 doit1 'my option' => efidebug boot add 00a2 label2 scsi 0:1 doit2 => efidebug boot dump Boot00A0: attributes: A-- (0x00000001) label: label1 file_path: .../HD(1,MBR,0xeac4e18b,0x800,0x3fffe)/doit1 data: 00000000: 6d 00 79 00 20 00 6f 00 70 00 74 00 69 00 6f 00 m.y. .o.p.t.i.o. 00000010: 6e 00 00 00 n... Boot00A1: attributes: A-- (0x00000001) label: label2 file_path: .../HD(1,MBR,0xeac4e18b,0x800,0x3fffe)/doit2 data: Signed-off-by: Heinrich Schuchardt --- cmd/efidebug.c | 27 +++++++++++++++++---------- include/efi_loader.h | 2 +- lib/efi_loader/efi_bootmgr.c | 15 ++++++++------- 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/cmd/efidebug.c b/cmd/efidebug.c index efe4ea873e..c4ac9dd634 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -545,7 +546,10 @@ static int do_efi_boot_add(cmd_tbl_t *cmdtp, int flag, + sizeof(struct efi_device_path); /* for END */ /* optional data */ - lo.optional_data = (u8 *)(argc == 6 ? "" : argv[6]); + if (argc < 6) + lo.optional_data = NULL; + else + lo.optional_data = (const u8 *)argv[6]; size = efi_serialize_load_option(&lo, (u8 **)&data); if (!size) { @@ -615,12 +619,13 @@ static int do_efi_boot_rm(cmd_tbl_t *cmdtp, int flag, /** * show_efi_boot_opt_data() - dump UEFI load option * - * @id: Load option number - * @data: Value of UEFI load option variable + * @id: load option number + * @data: value of UEFI load option variable + * @size: size of the boot option * * Decode the value of UEFI load option variable and print information. */ -static void show_efi_boot_opt_data(int id, void *data) +static void show_efi_boot_opt_data(int id, void *data, size_t size) { struct efi_load_option lo; char *label, *p; @@ -638,7 +643,7 @@ static void show_efi_boot_opt_data(int id, void *data) utf16_utf8_strncpy(&p, lo.label, label_len16); printf("Boot%04X:\n", id); - printf("\tattributes: %c%c%c (0x%08x)\n", + printf(" attributes: %c%c%c (0x%08x)\n", /* ACTIVE */ lo.attributes & LOAD_OPTION_ACTIVE ? 'A' : '-', /* FORCE RECONNECT */ @@ -646,14 +651,16 @@ static void show_efi_boot_opt_data(int id, void *data) /* HIDDEN */ lo.attributes & LOAD_OPTION_HIDDEN ? 'H' : '-', lo.attributes); - printf("\tlabel: %s\n", label); + printf(" label: %s\n", label); dp_str = efi_dp_str(lo.file_path); - printf("\tfile_path: %ls\n", dp_str); + printf(" file_path: %ls\n", dp_str); efi_free_pool(dp_str); - printf("\tdata: %s\n", lo.optional_data); - + printf(" data:\n"); + print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1, + lo.optional_data, size + (u8 *)data - + (u8 *)lo.optional_data, true); free(label); } @@ -686,7 +693,7 @@ static void show_efi_boot_opt(int id) data)); } if (ret == EFI_SUCCESS) - show_efi_boot_opt_data(id, data); + show_efi_boot_opt_data(id, data, size); else if (ret == EFI_NOT_FOUND) printf("Boot%04X: not found\n", id); diff --git a/include/efi_loader.h b/include/efi_loader.h index 39ed8a6fa5..07b913d256 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -560,7 +560,7 @@ struct efi_load_option { u16 file_path_length; u16 *label; struct efi_device_path *file_path; - u8 *optional_data; + const u8 *optional_data; }; void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data); diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 4ccba22875..7bf51874c1 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -53,19 +53,20 @@ void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data) */ unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data) { - unsigned long label_len, option_len; + unsigned long label_len; unsigned long size; u8 *p; label_len = (u16_strlen(lo->label) + 1) * sizeof(u16); - option_len = strlen((char *)lo->optional_data); /* total size */ size = sizeof(lo->attributes); size += sizeof(lo->file_path_length); size += label_len; size += lo->file_path_length; - size += option_len + 1; + if (lo->optional_data) + size += (utf8_utf16_strlen((const char *)lo->optional_data) + + 1) * sizeof(u16); p = malloc(size); if (!p) return 0; @@ -84,10 +85,10 @@ unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data) memcpy(p, lo->file_path, lo->file_path_length); p += lo->file_path_length; - memcpy(p, lo->optional_data, option_len); - p += option_len; - *(char *)p = '\0'; - + if (lo->optional_data) { + utf8_utf16_strcpy((u16 **)&p, (const char *)lo->optional_data); + p += sizeof(u16); /* size of trailing \0 */ + } return size; } -- cgit v1.2.3 From 45203e0ccbfaab5eafe3b6a7b8bb742182a83077 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 30 Apr 2019 07:14:09 +0200 Subject: efi_loader: memory leak in append value When printing an UEFI variable an error may arise while converting an illegal hexadecimal value. In this case a buffer is leaked. Close the memory leak. Provide an error message. Reported-by: Coverity (CID 185830) Signed-off-by: Heinrich Schuchardt --- cmd/nvedit_efi.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cmd/nvedit_efi.c b/cmd/nvedit_efi.c index e65b38dbf3..2805e8182b 100644 --- a/cmd/nvedit_efi.c +++ b/cmd/nvedit_efi.c @@ -291,8 +291,11 @@ static int append_value(char **bufp, size_t *sizep, char *data) if (!tmp_buf) return -1; - if (hex2bin((u8 *)tmp_buf, data, len) < 0) + if (hex2bin((u8 *)tmp_buf, data, len) < 0) { + printf("Error: illegal hexadecimal string\n"); + free(tmp_buf); return -1; + } value = tmp_buf; } else { /* string */ -- cgit v1.2.3 From 556d8dc937f74fa8a5fa849b7e1393db3446f3b2 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 30 Apr 2019 17:57:30 +0200 Subject: efi_loader: implement support of exit data In case of a failure exit data may be passed to Exit() which in turn is returned by StartImage(). Let the `bootefi` command print the exit data string in case of an error. Signed-off-by: Heinrich Schuchardt --- cmd/bootefi.c | 11 +++++++--- include/efi_loader.h | 5 +++++ lib/efi_loader/efi_boottime.c | 47 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/cmd/bootefi.c b/cmd/bootefi.c index b93d8c6a32..f1d7d8bc66 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -297,6 +297,8 @@ static efi_status_t efi_install_fdt(const char *fdt_opt) static efi_status_t do_bootefi_exec(efi_handle_t handle) { efi_status_t ret; + efi_uintn_t exit_data_size = 0; + u16 *exit_data = NULL; /* Transfer environment variable as load options */ ret = set_load_options(handle, "bootargs"); @@ -304,7 +306,12 @@ static efi_status_t do_bootefi_exec(efi_handle_t handle) return ret; /* Call our payload! */ - ret = EFI_CALL(efi_start_image(handle, NULL, NULL)); + ret = EFI_CALL(efi_start_image(handle, &exit_data_size, &exit_data)); + printf("## Application terminated, r = %lu\n", ret & ~EFI_ERROR_MASK); + if (ret && exit_data) { + printf("## %ls\n", exit_data); + efi_free_pool(exit_data); + } efi_restore_gd(); @@ -357,7 +364,6 @@ static int do_efibootmgr(const char *fdt_opt) } ret = do_bootefi_exec(handle); - printf("## Application terminated, r = %lu\n", ret & ~EFI_ERROR_MASK); if (ret != EFI_SUCCESS) return CMD_RET_FAILURE; @@ -472,7 +478,6 @@ static int do_bootefi_image(const char *image_opt, const char *fdt_opt) goto out; ret = do_bootefi_exec(handle); - printf("## Application terminated, r = %lu\n", ret & ~EFI_ERROR_MASK); out: if (mem_handle) diff --git a/include/efi_loader.h b/include/efi_loader.h index 07b913d256..7af3f16ef8 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -207,12 +207,17 @@ struct efi_object { * struct efi_loaded_image_obj - handle of a loaded image * * @header: EFI object header + * @exit_status: exit status passed to Exit() + * @exit_data_size: exit data size passed to Exit() + * @exit_data: exit data passed to Exit() * @exit_jmp: long jump buffer for returning form started image * @entry: entry address of the relocated image */ struct efi_loaded_image_obj { struct efi_object header; efi_status_t exit_status; + efi_uintn_t *exit_data_size; + u16 **exit_data; struct jmp_buf_data exit_jmp; EFIAPI efi_status_t (*entry)(efi_handle_t image_handle, struct efi_system_table *st); diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index a72cbe1559..6d2dc2dda3 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -2626,6 +2626,9 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, efi_is_direct_boot = false; + image_obj->exit_data_size = exit_data_size; + image_obj->exit_data = exit_data; + /* call the image! */ if (setjmp(&image_obj->exit_jmp)) { /* @@ -2669,6 +2672,41 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, return EFI_CALL(systab.boottime->exit(image_handle, ret, 0, NULL)); } +/** + * efi_update_exit_data() - fill exit data parameters of StartImage() + * + * @image_obj image handle + * @exit_data_size size of the exit data buffer + * @exit_data buffer with data returned by UEFI payload + * Return: status code + */ +static efi_status_t efi_update_exit_data(struct efi_loaded_image_obj *image_obj, + efi_uintn_t exit_data_size, + u16 *exit_data) +{ + efi_status_t ret; + + /* + * If exit_data is not provided to StartImage(), exit_data_size must be + * ignored. + */ + if (!image_obj->exit_data) + return EFI_SUCCESS; + if (image_obj->exit_data_size) + *image_obj->exit_data_size = exit_data_size; + if (exit_data_size && exit_data) { + ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, + exit_data_size, + (void **)image_obj->exit_data); + if (ret != EFI_SUCCESS) + return ret; + memcpy(*image_obj->exit_data, exit_data, exit_data_size); + } else { + image_obj->exit_data = NULL; + } + return EFI_SUCCESS; +} + /** * efi_exit() - leave an EFI application or driver * @image_handle: handle of the application or driver that is exiting @@ -2709,6 +2747,15 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, if (ret != EFI_SUCCESS) goto out; + /* Exit data is only foreseen in case of failure. */ + if (exit_status != EFI_SUCCESS) { + ret = efi_update_exit_data(image_obj, exit_data_size, + exit_data); + /* Exiting has priority. Don't return error to caller. */ + if (ret != EFI_SUCCESS) + EFI_PRINT("%s: out of memory\n", __func__); + } + /* Make sure entry/exit counts for EFI world cross-overs match */ EFI_EXIT(exit_status); -- cgit v1.2.3 From a9a25cc3e7d817048e831bfc70ceee11d761ed4f Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sun, 30 Sep 2018 13:26:36 +0200 Subject: efi_selftest: test exit_data Amend the unit test 'start image exit' to transfer a string as exit data. Signed-off-by: Heinrich Schuchardt --- include/efi_selftest.h | 2 +- lib/efi_selftest/efi_selftest_miniapp_exit.c | 17 ++++++++++++----- lib/efi_selftest/efi_selftest_startimage_exit.c | 15 ++++++++++++++- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/include/efi_selftest.h b/include/efi_selftest.h index 49d3d6d0b4..dd42e49023 100644 --- a/include/efi_selftest.h +++ b/include/efi_selftest.h @@ -16,7 +16,7 @@ #define EFI_ST_SUCCESS 0 #define EFI_ST_FAILURE 1 - +#define EFI_ST_SUCCESS_STR L"SUCCESS" /* * Prints a message. */ diff --git a/lib/efi_selftest/efi_selftest_miniapp_exit.c b/lib/efi_selftest/efi_selftest_miniapp_exit.c index b3ca109d81..6b5cfb01cf 100644 --- a/lib/efi_selftest/efi_selftest_miniapp_exit.c +++ b/lib/efi_selftest/efi_selftest_miniapp_exit.c @@ -9,7 +9,7 @@ */ #include -#include +#include static efi_guid_t loaded_image_protocol_guid = EFI_LOADED_IMAGE_PROTOCOL_GUID; @@ -66,15 +66,22 @@ 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; - efi_status_t ret = EFI_UNSUPPORTED; + efi_status_t ret; + u16 text[] = EFI_ST_SUCCESS_STR; con_out->output_string(con_out, L"EFI application calling Exit\n"); - if (check_loaded_image_protocol(handle, systable) != EFI_SUCCESS) + if (check_loaded_image_protocol(handle, systable) != EFI_SUCCESS) { + con_out->output_string(con_out, + L"Loaded image protocol missing\n"); ret = EFI_NOT_FOUND; + goto out; + } - /* The return value is checked by the calling test */ - systable->boottime->exit(handle, ret, 0, NULL); + /* This return value is expected by the calling test */ + ret = EFI_UNSUPPORTED; +out: + systable->boottime->exit(handle, ret, sizeof(text), text); /* * This statement should not be reached. diff --git a/lib/efi_selftest/efi_selftest_startimage_exit.c b/lib/efi_selftest/efi_selftest_startimage_exit.c index fa4b7d4a9b..96049dea86 100644 --- a/lib/efi_selftest/efi_selftest_startimage_exit.c +++ b/lib/efi_selftest/efi_selftest_startimage_exit.c @@ -123,6 +123,9 @@ static int execute(void) { efi_status_t ret; efi_handle_t handle; + efi_uintn_t exit_data_size = 0; + u16 *exit_data = NULL; + u16 expected_text[] = EFI_ST_SUCCESS_STR; ret = boottime->load_image(false, image_handle, NULL, image, img.length, &handle); @@ -130,11 +133,21 @@ static int execute(void) efi_st_error("Failed to load image\n"); return EFI_ST_FAILURE; } - ret = boottime->start_image(handle, NULL, NULL); + ret = boottime->start_image(handle, &exit_data_size, &exit_data); if (ret != EFI_UNSUPPORTED) { efi_st_error("Wrong return value from application\n"); return EFI_ST_FAILURE; } + if (!exit_data || exit_data_size != sizeof(expected_text) || + efi_st_memcmp(exit_data, expected_text, sizeof(expected_text))) { + efi_st_error("Incorrect exit data\n"); + return EFI_ST_FAILURE; + } + ret = boottime->free_pool(exit_data); + if (ret != EFI_SUCCESS) { + efi_st_error("Failed to free exit data\n"); + return EFI_ST_FAILURE; + } return EFI_ST_SUCCESS; } -- cgit v1.2.3 From e6023be41e94eb2bab4c93d343ac3deddfe12c7b Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 1 May 2019 09:42:39 +0200 Subject: efi_loader: description of efi_add_handle() Correct the comments describing function efi_add_handle(). Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_boottime.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 6d2dc2dda3..e5c46e9f08 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -423,10 +423,12 @@ static efi_status_t EFIAPI efi_free_pool_ext(void *buffer) } /** - * efi_add_handle() - add a new object to the object list - * @obj: object to be added + * efi_add_handle() - add a new handle to the object list * - * The protocols list is initialized. The object handle is set. + * @handle: handle to be added + * + * The protocols list is initialized. The handle is added to the list of known + * UEFI objects. */ void efi_add_handle(efi_handle_t handle) { -- cgit v1.2.3 From 1cfe9694752eb638bcf766429bc64cad2dbde041 Mon Sep 17 00:00:00 2001 From: Eugeniu Rosca Date: Tue, 30 Apr 2019 04:53:44 +0200 Subject: disk: efi: Fix memory leak on 'gpt guid' Below is what happens on R-Car H3ULCB-KF using clean U-Boot v2019.04-00810-g6aebc0d11a10 and r8a7795_ulcb_defconfig: => ### interrupt autoboot => gpt guid mmc 1 21200400-0804-0146-9dcc-a8c51255994f success! => ### keep calling 'gpt guid mmc 1' => ### on 59th call, we are out of memory: => gpt guid mmc 1 alloc_read_gpt_entries: ERROR: Can't allocate 0X4000 bytes for GPT Entries GPT: Failed to allocate memory for PTE get_disk_guid: *** ERROR: Invalid GPT *** alloc_read_gpt_entries: ERROR: Can't allocate 0X4000 bytes for GPT Entries GPT: Failed to allocate memory for PTE get_disk_guid: *** ERROR: Invalid Backup GPT *** error! After some inspection, it looks like get_disk_guid(), added via v2017.09 commit 73d6d18b7147c9 ("GPT: add accessor function for disk GUID"), unlike other callers of is_gpt_valid(), doesn't free the memory pointed out by 'gpt_entry *gpt_pte'. The latter is allocated by is_gpt_valid() via alloc_read_gpt_entries(). With the fix applied, the reproduction scenario has been run hundreds of times ('while true; do gpt guid mmc 1; done') w/o running into OOM. Fixes: 73d6d18b7147c9 ("GPT: add accessor function for disk GUID") Signed-off-by: Eugeniu Rosca Reviewed-by: Heinrich Schuchardt --- disk/part_efi.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/disk/part_efi.c b/disk/part_efi.c index 239455b816..812d14cdd8 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -209,6 +209,8 @@ int get_disk_guid(struct blk_desc * dev_desc, char *guid) guid_bin = gpt_head->disk_guid.b; uuid_bin_to_str(guid_bin, guid, UUID_STR_FORMAT_GUID); + /* Remember to free pte */ + free(gpt_pte); return 0; } -- cgit v1.2.3 From 716f919d2da723fae9416ea4ec461c1e29e71de0 Mon Sep 17 00:00:00 2001 From: Eugeniu Rosca Date: Tue, 30 Apr 2019 04:53:45 +0200 Subject: disk: efi: Fix memory leak on 'gpt verify' Below is what happens on R-Car H3ULCB-KF using clean U-Boot v2019.04-00810-g6aebc0d11a10 and r8a7795_ulcb_defconfig: => ### interrupt autoboot => gpt verify mmc 1 No partition list provided - only basic check Verify GPT: success! => ### keep calling 'gpt verify mmc 1' => ### on 58th call, we are out of memory: => gpt verify mmc 1 alloc_read_gpt_entries: ERROR: Can't allocate 0X4000 bytes for GPT Entries GPT: Failed to allocate memory for PTE gpt_verify_headers: *** ERROR: Invalid Backup GPT *** Verify GPT: error! This is caused by calling is_gpt_valid() twice (hence allocating pte also twice via alloc_read_gpt_entries()) while freeing pte only _once_ in the caller of gpt_verify_headers(). Fix that by freeing the pte allocated and populated for primary GPT _before_ allocating and populating the pte for backup GPT. The latter will be freed by the caller of gpt_verify_headers(). With the fix applied, the reproduction scenario [1-2] has been run hundreds of times in a loop w/o running into OOM. [1] gpt verify mmc 1 [2] gpt verify mmc 1 $partitions Fixes: cef68bf9042dda ("gpt: part: Definition and declaration of GPT verification functions") Signed-off-by: Eugeniu Rosca Reviewed-by: Heinrich Schuchardt --- disk/part_efi.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/disk/part_efi.c b/disk/part_efi.c index 812d14cdd8..c0fa753339 100644 --- a/disk/part_efi.c +++ b/disk/part_efi.c @@ -698,6 +698,10 @@ int gpt_verify_headers(struct blk_desc *dev_desc, gpt_header *gpt_head, __func__); return -1; } + + /* Free pte before allocating again */ + free(*gpt_pte); + if (is_gpt_valid(dev_desc, (dev_desc->lba - 1), gpt_head, gpt_pte) != 1) { printf("%s: *** ERROR: Invalid Backup GPT ***\n", -- cgit v1.2.3 From d02660960d78a3d07bdf23dd3b33300779cf74bb Mon Sep 17 00:00:00 2001 From: Eugeniu Rosca Date: Tue, 30 Apr 2019 04:53:46 +0200 Subject: cmd: gpt: fix and tidy up help message Apply the following changes: - Guard the 'gpt read' command by 'ifdef CONFIG_CMD_GPT_RENAME', since 'gpt read' is not available on CMD_GPT_RENAME=n - Prefix the {read,swap,rename} commands with one space for consistency - Prefix the 'guid' commands with 'gpt' for consistency Signed-off-by: Eugeniu Rosca Reviewed-by: Heinrich Schuchardt --- cmd/gpt.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/gpt.c b/cmd/gpt.c index 638870352f..33cda51396 100644 --- a/cmd/gpt.c +++ b/cmd/gpt.c @@ -876,21 +876,21 @@ U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt, " Example usage:\n" " gpt write mmc 0 $partitions\n" " gpt verify mmc 0 $partitions\n" - " read \n" - " - read GPT into a data structure for manipulation\n" - " guid \n" + " gpt guid \n" " - print disk GUID\n" - " guid \n" + " gpt guid \n" " - set environment variable to disk GUID\n" " Example usage:\n" " gpt guid mmc 0\n" " gpt guid mmc 0 varname\n" #ifdef CONFIG_CMD_GPT_RENAME "gpt partition renaming commands:\n" - "gpt swap \n" + " gpt read \n" + " - read GPT into a data structure for manipulation\n" + " gpt swap \n" " - change all partitions named name1 to name2\n" " and vice-versa\n" - "gpt rename \n" + " gpt rename \n" " - rename the specified partition\n" " Example usage:\n" " gpt swap mmc 0 foo bar\n" -- cgit v1.2.3 From 4ccf678f37731d8ec09eae8dca5f4cbe84132a52 Mon Sep 17 00:00:00 2001 From: Eugeniu Rosca Date: Thu, 2 May 2019 14:27:06 +0200 Subject: lib: uuid: Fix unseeded PRNG on RANDOM_UUID=y The random uuid values (enabled via CONFIG_RANDOM_UUID=y) on our platform are always the same. Below is consistent on each cold boot: => ### interrupt autoboot => env default -a; gpt write mmc 1 $partitions; print uuid_gpt_misc ... uuid_gpt_misc=d117f98e-6f2c-d04b-a5b2-331a19f91cb2 => env default -a; gpt write mmc 1 $partitions; print uuid_gpt_misc ... uuid_gpt_misc=ad5ec4b6-2d9f-8544-9417-fe3bd1c9b1b3 => env default -a; gpt write mmc 1 $partitions; print uuid_gpt_misc ... uuid_gpt_misc=cceb0b18-39cb-d547-9db7-03b405fa77d4 => env default -a; gpt write mmc 1 $partitions; print uuid_gpt_misc ... uuid_gpt_misc=d4981a2b-0478-544e-9607-7fd3c651068d => env default -a; gpt write mmc 1 $partitions; print uuid_gpt_misc ... uuid_gpt_misc=6d6c9a36-e919-264d-a9ee-bd00379686c7 While the uuids do change on every 'gpt write' command, the values appear to be taken from the same pool, in the same order. Assuming U-Boot with RANDOM_UUID=y is deployed on a large number of devices, all those devices would essentially expose the same UUID, breaking the assumption of system/RFS/application designers who rely on UUID as being globally unique (e.g. a database using UUID as key would alias/mix up entries/records due to duplicated UUID). The root cause seems to be simply _not_ seeding PRNG before generating a random value. It turns out this belongs to an established class of PRNG-specific problems, commonly known as "unseeded randomness", for which I am able to find below bugs/CVE/CWE: - https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2015-0285 ("CVE-2015-0285 openssl: handshake with unseeded PRNG") - https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2015-9019 ("CVE-2015-9019 libxslt: math.random() in xslt uses unseeded randomness") - https://cwe.mitre.org/data/definitions/336.html ("CWE-336: Same Seed in Pseudo-Random Number Generator (PRNG)") The first revision [1] of this patch updated the seed based on the output of get_timer(), similar to [4]. There are two problems with this approach: - get_timer() has a poor _ms_ resolution - when gen_rand_uuid() is called in a loop, get_timer() returns the same result, leading to the same seed being passed to srand(), leading to the same uuid being generated for several partitions with different names The above drawbacks have been addressed in the second version [2]. In its third revision (current), the patch reworded the description and summary line to emphasize it is a *fix* rather than an improvement. Testing [3] consisted of running 'gpt write mmc 1 $partitions' in a loop on R-Car3 for several minutes, collecting 8844 randomly generated UUIDS. Two consecutive cold boots are concatenated in the log. As a result, all uuid values are unique (scripted check). Thanks to Roman, who reported the issue and provided support in fixing. [1] https://patchwork.ozlabs.org/patch/1091802/ [2] https://patchwork.ozlabs.org/patch/1092945/ [3] https://gist.github.com/erosca/2820be9d554f76b982edd48474d0e7ca [4] commit da384a9d7628 ("net: rename and refactor eth_rand_ethaddr() function") Reported-by: Roman Stratiienko Signed-off-by: Eugeniu Rosca Reviewed-by: Heinrich Schuchardt --- lib/uuid.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/uuid.c b/lib/uuid.c index fa20ee39fc..2d4d6ef7e4 100644 --- a/lib/uuid.c +++ b/lib/uuid.c @@ -238,6 +238,8 @@ void gen_rand_uuid(unsigned char *uuid_bin) unsigned int *ptr = (unsigned int *)&uuid; int i; + srand(get_ticks() + rand()); + /* Set all fields randomly */ for (i = 0; i < sizeof(struct uuid) / sizeof(*ptr); i++) *(ptr + i) = cpu_to_be32(rand()); -- cgit v1.2.3