From 3ce7829792c50d1e5add3d8ef88883e8298aa4eb Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sat, 13 Oct 2018 20:52:08 -0700 Subject: efi_loader: fix relocation on x86_64 Currently the relocation of the EFI runtime on x86_64 fails. This renders the EFI subsystem unusable. The ELF relocation records for x86_64 contain an addend field. Always write the function name into error messages related to the EFI runtime relocation. Break an excessively long line. Signed-off-by: Heinrich Schuchardt Reviewed-by: Bin Meng Tested-by: Bin Meng Signed-off-by: Bin Meng Signed-off-by: Alexander Graf --- lib/efi_loader/efi_runtime.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'lib/efi_loader') diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index c5fbd91fa38..f059dc97fd4 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -41,9 +41,13 @@ static efi_status_t __efi_runtime EFIAPI efi_invalid_parameter(void); #elif defined(__arm__) #define R_RELATIVE R_ARM_RELATIVE #define R_MASK 0xffULL -#elif defined(__x86_64__) || defined(__i386__) +#elif defined(__i386__) #define R_RELATIVE R_386_RELATIVE #define R_MASK 0xffULL +#elif defined(__x86_64__) +#define R_RELATIVE R_X86_64_RELATIVE +#define R_MASK 0xffffffffULL +#define IS_RELA 1 #elif defined(__riscv) #define R_RELATIVE R_RISCV_RELATIVE #define R_MASK 0xffULL @@ -358,7 +362,8 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map) p = (void*)((ulong)rel->offset - base) + gd->relocaddr; - debug("%s: rel->info=%#lx *p=%#lx rel->offset=%p\n", __func__, rel->info, *p, rel->offset); + debug("%s: rel->info=%#lx *p=%#lx rel->offset=%p\n", __func__, + rel->info, *p, rel->offset); switch (rel->info & R_MASK) { case R_RELATIVE: @@ -377,6 +382,9 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map) } #endif default: + if (!efi_runtime_tobedetached(p)) + printf("%s: Unknown relocation type %llx\n", + __func__, rel->info & R_MASK); continue; } @@ -385,8 +393,8 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map) newaddr > (map->virtual_start + (map->num_pages << EFI_PAGE_SHIFT)))) { if (!efi_runtime_tobedetached(p)) - printf("U-Boot EFI: Relocation at %p is out of " - "range (%lx)\n", p, newaddr); + printf("%s: Relocation at %p is out of " + "range (%lx)\n", __func__, p, newaddr); continue; } -- cgit v1.3.1 From 0801d4d2fbceb04b4f1983fdd7c2dd5ae728fd74 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sun, 7 Oct 2018 05:26:26 +0200 Subject: efi_loader: correct signature of GetPosition, SetPosition The UEFI spec requires that file positions are passed as u64 in GetPosition() and SetPosition(). Check if the file handle points to a directory in GetPosition(). Provide a unit test for GetPosition() and SetPosition(). Fix Coverity warning CID 184079 (CONSTANT_EXPRESSION_RESULT). Add comments. Fixes: b6dd57773719 ("efi_loader: use correct types in EFI_FILE_PROTOCOL") Signed-off-by: Heinrich Schuchardt Signed-off-by: Alexander Graf --- include/efi_api.h | 4 +-- lib/efi_loader/efi_file.c | 39 ++++++++++++++++++++++------ lib/efi_selftest/efi_selftest_block_device.c | 30 +++++++++++++++++++-- 3 files changed, 61 insertions(+), 12 deletions(-) (limited to 'lib/efi_loader') diff --git a/include/efi_api.h b/include/efi_api.h index bea19a5a123..e850b951eb0 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -914,9 +914,9 @@ struct efi_file_handle { efi_status_t (EFIAPI *write)(struct efi_file_handle *file, efi_uintn_t *buffer_size, void *buffer); efi_status_t (EFIAPI *getpos)(struct efi_file_handle *file, - efi_uintn_t *pos); + u64 *pos); efi_status_t (EFIAPI *setpos)(struct efi_file_handle *file, - efi_uintn_t pos); + u64 pos); efi_status_t (EFIAPI *getinfo)(struct efi_file_handle *file, const efi_guid_t *info_type, efi_uintn_t *buffer_size, void *buffer); diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c index 0753a36a20b..c1e285e9f7c 100644 --- a/lib/efi_loader/efi_file.c +++ b/lib/efi_loader/efi_file.c @@ -436,28 +436,51 @@ error: return EFI_EXIT(ret); } +/** + * efi_file_getpos() - get current position in file + * + * This function implements the GetPosition service of the EFI file protocol. + * See the UEFI spec for details. + * + * @file: file handle + * @pos: pointer to file position + * Return: status code + */ static efi_status_t EFIAPI efi_file_getpos(struct efi_file_handle *file, - efi_uintn_t *pos) + u64 *pos) { + efi_status_t ret = EFI_SUCCESS; struct file_handle *fh = to_fh(file); EFI_ENTRY("%p, %p", file, pos); - if (fh->offset <= SIZE_MAX) { - *pos = fh->offset; - return EFI_EXIT(EFI_SUCCESS); - } else { - return EFI_EXIT(EFI_DEVICE_ERROR); + if (fh->isdir) { + ret = EFI_UNSUPPORTED; + goto out; } + + *pos = fh->offset; +out: + return EFI_EXIT(ret); } +/** + * efi_file_setpos() - set current position in file + * + * This function implements the SetPosition service of the EFI file protocol. + * See the UEFI spec for details. + * + * @file: file handle + * @pos: new file position + * Return: status code + */ static efi_status_t EFIAPI efi_file_setpos(struct efi_file_handle *file, - efi_uintn_t pos) + u64 pos) { struct file_handle *fh = to_fh(file); efi_status_t ret = EFI_SUCCESS; - EFI_ENTRY("%p, %zu", file, pos); + EFI_ENTRY("%p, %llu", file, pos); if (fh->isdir) { if (pos != 0) { diff --git a/lib/efi_selftest/efi_selftest_block_device.c b/lib/efi_selftest/efi_selftest_block_device.c index 1cd13042e90..d4e4fac1c74 100644 --- a/lib/efi_selftest/efi_selftest_block_device.c +++ b/lib/efi_selftest/efi_selftest_block_device.c @@ -308,6 +308,7 @@ static int execute(void) } system_info; efi_uintn_t buf_size; char buf[16] __aligned(ARCH_DMA_MINALIGN); + u64 pos; /* Connect controller to virtual disk */ ret = boottime->connect_controller(disk_handle, NULL, NULL, 1); @@ -392,21 +393,36 @@ static int execute(void) efi_st_error("Failed to open file\n"); return EFI_ST_FAILURE; } + ret = file->setpos(file, 1); + if (ret != EFI_SUCCESS) { + efi_st_error("SetPosition failed\n"); + return EFI_ST_FAILURE; + } buf_size = sizeof(buf) - 1; ret = file->read(file, &buf_size, buf); if (ret != EFI_SUCCESS) { efi_st_error("Failed to read file\n"); return EFI_ST_FAILURE; } - if (buf_size != 13) { + if (buf_size != 12) { efi_st_error("Wrong number of bytes read: %u\n", (unsigned int)buf_size); return EFI_ST_FAILURE; } - if (efi_st_memcmp(buf, "Hello world!", 12)) { + if (efi_st_memcmp(buf, "ello world!", 11)) { efi_st_error("Unexpected file content\n"); return EFI_ST_FAILURE; } + ret = file->getpos(file, &pos); + if (ret != EFI_SUCCESS) { + efi_st_error("GetPosition failed\n"); + return EFI_ST_FAILURE; + } + if (pos != 13) { + efi_st_error("GetPosition returned %u, expected 13\n", + (unsigned int)pos); + return EFI_ST_FAILURE; + } ret = file->close(file); if (ret != EFI_SUCCESS) { efi_st_error("Failed to close file\n"); @@ -434,6 +450,16 @@ static int execute(void) efi_st_error("Failed to close file\n"); return EFI_ST_FAILURE; } + ret = file->getpos(file, &pos); + if (ret != EFI_SUCCESS) { + efi_st_error("GetPosition failed\n"); + return EFI_ST_FAILURE; + } + if (pos != 7) { + efi_st_error("GetPosition returned %u, expected 7\n", + (unsigned int)pos); + return EFI_ST_FAILURE; + } /* Verify file */ boottime->set_mem(buf, sizeof(buf), 0); -- cgit v1.3.1 From eee6530ed1a8174d0f60e4c3c86bea3274c95951 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 3 Oct 2018 20:02:29 +0200 Subject: efi_loader: efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES, ...) The first parameter of efi_allocate_pool is a memory type. It cannot be EFI_ALLOCATE_ANY_PAGES. Use EFI_BOOT_SERVICES_DATA instead. Signed-off-by: Heinrich Schuchardt Signed-off-by: Alexander Graf --- lib/efi_loader/efi_boottime.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'lib/efi_loader') diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 97eb19cd14d..fddfeb1a1a1 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -2023,7 +2023,7 @@ static efi_status_t EFIAPI efi_open_protocol_information( /* Copy entries */ buffer_size = count * sizeof(struct efi_open_protocol_info_entry); - r = efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES, buffer_size, + r = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, buffer_size, (void **)entry_buffer); if (r != EFI_SUCCESS) goto out; @@ -2080,7 +2080,7 @@ static efi_status_t EFIAPI efi_protocols_per_handle( size_t j = 0; buffer_size = sizeof(efi_guid_t *) * *protocol_buffer_count; - r = efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES, buffer_size, + r = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, buffer_size, (void **)protocol_buffer); if (r != EFI_SUCCESS) return EFI_EXIT(r); @@ -2133,7 +2133,7 @@ static efi_status_t EFIAPI efi_locate_handle_buffer( *buffer); if (r != EFI_BUFFER_TOO_SMALL) goto out; - r = efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES, buffer_size, + r = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, buffer_size, (void **)buffer); if (r != EFI_SUCCESS) goto out; -- cgit v1.3.1 From 6f566c231d93050e9366bc32067fdecd2aa8d8c6 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 2 Oct 2018 06:08:26 +0200 Subject: efi_loader: return type efi_console_register() Use a return type that can encompass the return value. This fixes CoverityScan CID 184090. Reported-by: Tom Rini Signed-off-by: Heinrich Schuchardt Signed-off-by: Alexander Graf --- include/efi_loader.h | 2 +- lib/efi_loader/efi_console.c | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) (limited to 'lib/efi_loader') diff --git a/include/efi_loader.h b/include/efi_loader.h index 34e44c6677c..1417c3588fb 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -236,7 +236,7 @@ efi_status_t efi_root_node_register(void); /* Called by bootefi to initialize runtime */ efi_status_t efi_initialize_system_table(void); /* Called by bootefi to make console interface available */ -int efi_console_register(void); +efi_status_t efi_console_register(void); /* Called by bootefi to make all disk storage accessible as EFI objects */ efi_status_t efi_disk_register(void); /* Create handles and protocols for the partitions of a block device */ diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c index 7ecdbb16669..0225222a616 100644 --- a/lib/efi_loader/efi_console.c +++ b/lib/efi_loader/efi_console.c @@ -1045,8 +1045,10 @@ static void EFIAPI efi_key_notify(struct efi_event *event, void *context) * efi_console_register() - install the console protocols * * This function is called from do_bootefi_exec(). + * + * Return: status code */ -int efi_console_register(void) +efi_status_t efi_console_register(void) { efi_status_t r; struct efi_object *efi_console_output_obj; -- cgit v1.3.1 From 2c61e0cc5c67345d2d3e4a66f6e22899afa381cc Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 2 Oct 2018 05:57:32 +0200 Subject: efi_loader: superfluous statement in is_dir() When is_dir() is called we have already execute set_blk_dev(fh). So don't call it again. This fixes CoverityScan CID 184093. Reported-by: Tom Rini Signed-off-by: Heinrich Schuchardt Signed-off-by: Alexander Graf --- lib/efi_loader/efi_file.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'lib/efi_loader') diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c index c1e285e9f7c..beb4fba917d 100644 --- a/lib/efi_loader/efi_file.c +++ b/lib/efi_loader/efi_file.c @@ -52,11 +52,18 @@ static int set_blk_dev(struct file_handle *fh) return fs_set_blk_dev_with_part(fh->fs->desc, fh->fs->part); } +/** + * is_dir() - check if file handle points to directory + * + * We assume that set_blk_dev(fh) has been called already. + * + * @fh: file handle + * Return: true if file handle points to a directory + */ static int is_dir(struct file_handle *fh) { struct fs_dir_stream *dirs; - set_blk_dev(fh); dirs = fs_opendir(fh->path); if (!dirs) return 0; -- cgit v1.3.1 From dadc2bddb0c0e00554666ec39bc10b9d66409f24 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Tue, 2 Oct 2018 05:30:05 +0200 Subject: efi_loader: memory leak in efi_set_variable() Do not leak native_name if out of memory. This addresses CoverityScan CID 184095. Reported-by: Tom Rini Signed-off-by: Heinrich Schuchardt Signed-off-by: Alexander Graf --- lib/efi_loader/efi_variable.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'lib/efi_loader') diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index a1313fa2158..19d9cb865f2 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -294,8 +294,10 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, efi_guid_t *vendor, } val = malloc(2 * data_size + strlen("{ro,run,boot}(blob)") + 1); - if (!val) - return EFI_EXIT(EFI_OUT_OF_RESOURCES); + if (!val) { + ret = EFI_OUT_OF_RESOURCES; + goto out; + } s = val; -- cgit v1.3.1 From 891dacf6731c3608715f8f6f07df6f9651aa5d4f Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Mon, 1 Oct 2018 05:24:46 +0200 Subject: efi_loader: remove lcd.h from efi_net.c Remove superfluous include. Signed-off-by: Heinrich Schuchardt Signed-off-by: Alexander Graf --- lib/efi_loader/efi_net.c | 1 - 1 file changed, 1 deletion(-) (limited to 'lib/efi_loader') diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c index 034d0d2ed03..4e8b2d597df 100644 --- a/lib/efi_loader/efi_net.c +++ b/lib/efi_loader/efi_net.c @@ -7,7 +7,6 @@ #include #include -#include #include static const efi_guid_t efi_net_guid = EFI_SIMPLE_NETWORK_GUID; -- cgit v1.3.1 From 1c3b2f4ae1da02f173315b1fc02b05122c02bd44 Mon Sep 17 00:00:00 2001 From: Tom Rini Date: Sun, 30 Sep 2018 10:38:15 -0400 Subject: efi_loader: Fix warning in efi_load_image() As observed with clang: lib/efi_loader/efi_boottime.c:1624:7: warning: variable 'info' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized] if (ret != EFI_SUCCESS) ^~~~~~~~~~~~~~~~~~ lib/efi_loader/efi_boottime.c:1653:7: note: uninitialized use occurs here free(info); ^~~~ lib/efi_loader/efi_boottime.c:1624:3: note: remove the 'if' if its condition is always false if (ret != EFI_SUCCESS) ^~~~~~~~~~~~~~~~~~~~~~~ lib/efi_loader/efi_boottime.c:1602:31: note: initialize the variable 'info' to silence this warning struct efi_loaded_image *info; ^ = NULL Rather than change how we unwind the function it makes the most sense to initialize info to NULL so that we can continue to pass it to free(). Fixes: c982874e930d ("efi_loader: refactor efi_setup_loaded_image()") Signed-off-by: Tom Rini Reviewed-by: Heinrich Schuchardt Signed-off-by: Alexander Graf --- lib/efi_loader/efi_boottime.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/efi_loader') diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index fddfeb1a1a1..6a07c361478 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1599,7 +1599,7 @@ static efi_status_t EFIAPI efi_load_image(bool boot_policy, efi_uintn_t source_size, efi_handle_t *image_handle) { - struct efi_loaded_image *info; + struct efi_loaded_image *info = NULL; struct efi_loaded_image_obj **image_obj = (struct efi_loaded_image_obj **)image_handle; efi_status_t ret; -- cgit v1.3.1 From 4f37fa470f440aaed392b640557a88113167ea86 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sun, 30 Sep 2018 13:40:43 +0200 Subject: efi_loader: fix typo in efi_boottime.c %s/conncected/connected/ Signed-off-by: Heinrich Schuchardt Signed-off-by: Alexander Graf --- lib/efi_loader/efi_boottime.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/efi_loader') diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 6a07c361478..da978d2b34a 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -2506,7 +2506,7 @@ static efi_status_t efi_protocol_open( if (item->info.attributes & EFI_OPEN_PROTOCOL_BY_DRIVER) opened_by_driver = true; } - /* Only one controller can be conncected */ + /* Only one controller can be connected */ if (opened_by_driver) return EFI_ACCESS_DENIED; } -- cgit v1.3.1 From d081f27fc28d3a0f9fbb6045e4121709bc303028 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 27 Sep 2018 20:55:04 +0200 Subject: efi_loader: efi_dp_get_next_instance() superfluous statement Remove a superfluous statement in efi_dp_get_next_instance(). Signed-off-by: Heinrich Schuchardt Signed-off-by: Alexander Graf --- lib/efi_loader/efi_device_path.c | 1 - 1 file changed, 1 deletion(-) (limited to 'lib/efi_loader') diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 5a61a1c1dcf..46a24f78824 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -382,7 +382,6 @@ struct efi_device_path *efi_dp_get_next_instance(struct efi_device_path **dp, *size = 0; if (!dp || !*dp) return NULL; - p = *dp; sz = efi_dp_instance_size(*dp); p = dp_alloc(sz + sizeof(END)); if (!p) -- cgit v1.3.1