From a07d493c9df9511399f97e74d64545126f969008 Mon Sep 17 00:00:00 2001 From: Mathieu Choplain Date: Tue, 28 May 2024 18:13:02 +0200 Subject: [PATCH] llext: relocate all symbols regardless of type In the current implementation, the LLEXT linker will only apply relocations targeting a given symbol if it has a specfic symbol type. This is overzealous and causes issues on some platforms, as some symbols that need to be relocated are skipped due to being of a "bad" type. Ignore the symbol type when performing relocation to solve this problem, but also add checks to ensure we don't attempt to relocate symbols with an invalid section index. If such a relocation is found, return an error instead of ignoring the relocation entry to ensure that it is impossible to execute code from a (partially) unrelocated LLEXT. Also remove all hacks added to circumvent this issue: * qemu_cortex_r5 exclusion from test cases * unnecessary exclusion of some flags when building with LLEXT EDK Fixes #72832. Signed-off-by: Mathieu Choplain --- include/zephyr/llext/elf.h | 2 ++ samples/subsys/llext/edk/app/CMakeLists.txt | 8 ----- subsys/llext/llext.c | 37 ++++++++++++++++----- tests/subsys/llext/simple/testcase.yaml | 1 - 4 files changed, 30 insertions(+), 18 deletions(-) diff --git a/include/zephyr/llext/elf.h b/include/zephyr/llext/elf.h index edf83cae271..7e8628b37d5 100644 --- a/include/zephyr/llext/elf.h +++ b/include/zephyr/llext/elf.h @@ -245,8 +245,10 @@ struct elf64_sym { }; #define SHN_UNDEF 0 +#define SHN_LORESERVE 0xff00 #define SHN_ABS 0xfff1 #define SHN_COMMON 0xfff2 +#define SHN_HIRESERVE 0xffff #define STT_NOTYPE 0 #define STT_OBJECT 1 diff --git a/samples/subsys/llext/edk/app/CMakeLists.txt b/samples/subsys/llext/edk/app/CMakeLists.txt index 1afc133186e..5e553869dc1 100644 --- a/samples/subsys/llext/edk/app/CMakeLists.txt +++ b/samples/subsys/llext/edk/app/CMakeLists.txt @@ -1,13 +1,5 @@ # SPDX-License-Identifier: Apache-2.0 -# It seems llext currently doesn't support some Thumb32 relocation -# instructions generated when building the extensions with default -# flags. As a workaround, we remove these *unrelated* flags from the -# default flags. This allows the extensions to be built without the -# unsupported instructions. -# See issue #72832 for more details. -list(APPEND LLEXT_EDK_REMOVE_FLAGS -mcpu=cortex-r5 -mfloat-abi=hard) - cmake_minimum_required(VERSION 3.20.0) find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) diff --git a/subsys/llext/llext.c b/subsys/llext/llext.c index ec145cdd4df..856b322c49a 100644 --- a/subsys/llext/llext.c +++ b/subsys/llext/llext.c @@ -874,18 +874,37 @@ static int llext_link(struct llext_loader *ldr, struct llext *ext, bool do_local } else { LOG_INF("found symbol %s at 0x%lx", name, link_addr); } - } else if (ELF_ST_TYPE(sym.st_info) == STT_SECTION || - ELF_ST_TYPE(sym.st_info) == STT_FUNC || - ELF_ST_TYPE(sym.st_info) == STT_OBJECT) { - /* Link address is relative to the start of the section */ + } else if (sym.st_shndx == SHN_ABS) { + /* Absolute symbol */ + link_addr = sym.st_value; + } else if ((sym.st_shndx < ldr->hdr.e_shnum) && + !IN_RANGE(sym.st_shndx, SHN_LORESERVE, SHN_HIRESERVE)) { + /* This check rejects all relocations whose target symbol + * has a section index higher than the maximum possible + * in this ELF file, or belongs in the reserved range: + * they will be caught by the `else` below and cause an + * error to be returned. This aborts the LLEXT's loading + * and prevents execution of improperly relocated code, + * which is dangerous. + * + * Note that the unsupported SHN_COMMON section is rejected + * as part of this check. Also note that SHN_ABS would be + * rejected as well, but we want to handle it properly: + * for this reason, this check must come AFTER handling + * the case where the symbol's section index is SHN_ABS! + * + * + * For regular symbols, the link address is obtained by + * adding st_value to the start address of the section + * in which the target symbol resides. + */ link_addr = (uintptr_t)ext->mem[ldr->sect_map[sym.st_shndx]] + sym.st_value; - - LOG_INF("found section symbol %s addr 0x%lx", name, link_addr); } else { - /* Nothing to relocate here */ - LOG_DBG("not relocated"); - continue; + LOG_ERR("rela section %d, entry %d: cannot apply relocation: " + "target symbol has unexpected section index %d (0x%X)", + i, j, sym.st_shndx, sym.st_shndx); + return -ENOEXEC; } LOG_INF("writing relocation symbol %s type %zd sym %zd at addr 0x%lx " diff --git a/tests/subsys/llext/simple/testcase.yaml b/tests/subsys/llext/simple/testcase.yaml index abea52981cb..6325bd51425 100644 --- a/tests/subsys/llext/simple/testcase.yaml +++ b/tests/subsys/llext/simple/testcase.yaml @@ -7,7 +7,6 @@ common: - apollo4p_evb - apollo4p_blue_kxr_evb - numaker_pfm_m487 # See #63167 - - qemu_cortex_r5 # unsupported relocations tests: # While there is in practice no value in compiling subsys/llext/*.c