From 53a179ac4e3c4d85b400adb17f98f23d4086a8cd Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Fri, 13 Jun 2025 10:47:13 +0200 Subject: [PATCH] llext: make unresolved symbol errors fatal With experience it becomes clear, that failing to resolve symbols during the linking process is likely fatal for the module loading and a simple warning isn't enough. Fail loading instead. Signed-off-by: Guennadi Liakhovetski --- arch/xtensa/core/elf.c | 34 +++++++++++++------------- include/zephyr/llext/llext.h | 12 ++++++---- subsys/llext/llext_link.c | 46 ++++++++++++++++++++++++++---------- 3 files changed, 58 insertions(+), 34 deletions(-) diff --git a/arch/xtensa/core/elf.c b/arch/xtensa/core/elf.c index d16f51e61a2..d0cf6281355 100644 --- a/arch/xtensa/core/elf.c +++ b/arch/xtensa/core/elf.c @@ -32,10 +32,10 @@ LOG_MODULE_DECLARE(llext, CONFIG_LLEXT_LOG_LEVEL); #define R_XTENSA_ASM_EXPAND 11 #define R_XTENSA_SLOT0_OP 20 -static void xtensa_elf_relocate(struct llext_loader *ldr, struct llext *ext, - const elf_rela_t *rel, uintptr_t addr, - uint8_t *loc, int type, uint32_t stb, - const struct llext_load_param *ldr_parm) +static int xtensa_elf_relocate(struct llext_loader *ldr, struct llext *ext, + const elf_rela_t *rel, uintptr_t addr, + uint8_t *loc, int type, uint32_t stb, + const struct llext_load_param *ldr_parm) { elf_word *got_entry = (elf_word *)loc; @@ -59,7 +59,7 @@ static void xtensa_elf_relocate(struct llext_loader *ldr, struct llext *ext, if (sh_ndx == ext->sect_cnt) { LOG_ERR("%#x not found in any of the sections", *got_entry); - return; + return -ENOENT; } *got_entry += (uintptr_t)llext_loaded_sect_ptr(ldr, ext, sh_ndx) - @@ -87,7 +87,7 @@ static void xtensa_elf_relocate(struct llext_loader *ldr, struct llext *ext, } if (ret) { LOG_ERR("Failed to read a symbol table entry, LLEXT linking might fail."); - return; + return ret; } /* @@ -124,19 +124,21 @@ static void xtensa_elf_relocate(struct llext_loader *ldr, struct llext *ext, default: LOG_DBG("Unsupported relocation type %u", type); - return; + return 0; } LOG_DBG("Applied relocation to %#x type %u at %p", *(uint32_t *)((uintptr_t)got_entry & ~3), type, (void *)got_entry); + + return 0; } /** * @brief Architecture specific function for STB_LOCAL ELF relocations */ -void arch_elf_relocate_local(struct llext_loader *ldr, struct llext *ext, const elf_rela_t *rel, - const elf_sym_t *sym, uint8_t *rel_addr, - const struct llext_load_param *ldr_parm) +int arch_elf_relocate_local(struct llext_loader *ldr, struct llext *ext, const elf_rela_t *rel, + const elf_sym_t *sym, uint8_t *rel_addr, + const struct llext_load_param *ldr_parm) { int type = ELF32_R_TYPE(rel->r_info); uintptr_t sh_addr; @@ -152,15 +154,15 @@ void arch_elf_relocate_local(struct llext_loader *ldr, struct llext *ext, const sh_addr = ldr->sects[LLEXT_MEM_TEXT].sh_addr; } - xtensa_elf_relocate(ldr, ext, rel, sh_addr, rel_addr, type, ELF_ST_BIND(sym->st_info), - ldr_parm); + return xtensa_elf_relocate(ldr, ext, rel, sh_addr, rel_addr, type, + ELF_ST_BIND(sym->st_info), ldr_parm); } /** * @brief Architecture specific function for STB_GLOBAL ELF relocations */ -void arch_elf_relocate_global(struct llext_loader *ldr, struct llext *ext, const elf_rela_t *rel, - const elf_sym_t *sym, uint8_t *rel_addr, const void *link_addr) +int arch_elf_relocate_global(struct llext_loader *ldr, struct llext *ext, const elf_rela_t *rel, + const elf_sym_t *sym, uint8_t *rel_addr, const void *link_addr) { int type = ELF32_R_TYPE(rel->r_info); @@ -169,6 +171,6 @@ void arch_elf_relocate_global(struct llext_loader *ldr, struct llext *ext, const LOG_WRN("global: non-zero relative value %#x", *(elf_word *)rel_addr); } - xtensa_elf_relocate(ldr, ext, rel, (uintptr_t)link_addr, rel_addr, type, - ELF_ST_BIND(sym->st_info), NULL); + return xtensa_elf_relocate(ldr, ext, rel, (uintptr_t)link_addr, rel_addr, type, + ELF_ST_BIND(sym->st_info), NULL); } diff --git a/include/zephyr/llext/llext.h b/include/zephyr/llext/llext.h index 887f2152f4f..17030334717 100644 --- a/include/zephyr/llext/llext.h +++ b/include/zephyr/llext/llext.h @@ -409,10 +409,11 @@ int llext_get_section_header(struct llext_loader *loader, struct llext *ext, * @param[in] sym Corresponding symbol table entry * @param[in] rel_addr Address where relocation should be performed * @param[in] ldr_parm Loader parameters + * @returns 0 on success or a negative error code */ -void arch_elf_relocate_local(struct llext_loader *loader, struct llext *ext, const elf_rela_t *rel, - const elf_sym_t *sym, uint8_t *rel_addr, - const struct llext_load_param *ldr_parm); +int arch_elf_relocate_local(struct llext_loader *loader, struct llext *ext, const elf_rela_t *rel, + const elf_sym_t *sym, uint8_t *rel_addr, + const struct llext_load_param *ldr_parm); /** * @brief Architecture specific function for global binding relocations @@ -423,9 +424,10 @@ void arch_elf_relocate_local(struct llext_loader *loader, struct llext *ext, con * @param[in] sym Corresponding symbol table entry * @param[in] rel_addr Address where relocation should be performed * @param[in] link_addr target address for table-based relocations + * @returns 0 on success or a negative error code */ -void arch_elf_relocate_global(struct llext_loader *loader, struct llext *ext, const elf_rela_t *rel, - const elf_sym_t *sym, uint8_t *rel_addr, const void *link_addr); +int arch_elf_relocate_global(struct llext_loader *loader, struct llext *ext, const elf_rela_t *rel, + const elf_sym_t *sym, uint8_t *rel_addr, const void *link_addr); /** * @brief Initialize LLEXT heap dynamically diff --git a/subsys/llext/llext_link.c b/subsys/llext/llext_link.c index 93bbbe3ee5e..d7b32011c34 100644 --- a/subsys/llext/llext_link.c +++ b/subsys/llext/llext_link.c @@ -32,16 +32,18 @@ __weak int arch_elf_relocate(struct llext_loader *ldr, struct llext *ext, elf_re return -ENOTSUP; } -__weak void arch_elf_relocate_local(struct llext_loader *ldr, struct llext *ext, +__weak int arch_elf_relocate_local(struct llext_loader *ldr, struct llext *ext, const elf_rela_t *rel, const elf_sym_t *sym, uint8_t *rel_addr, const struct llext_load_param *ldr_parm) { + return -ENOTSUP; } -__weak void arch_elf_relocate_global(struct llext_loader *ldr, struct llext *ext, - const elf_rela_t *rel, const elf_sym_t *sym, uint8_t *rel_addr, - const void *link_addr) +__weak int arch_elf_relocate_global(struct llext_loader *ldr, struct llext *ext, + const elf_rela_t *rel, const elf_sym_t *sym, uint8_t *rel_addr, + const void *link_addr) { + return -ENOTSUP; } /* @@ -246,8 +248,8 @@ int llext_lookup_symbol(struct llext_loader *ldr, struct llext *ext, uintptr_t * return 0; } -static void llext_link_plt(struct llext_loader *ldr, struct llext *ext, elf_shdr_t *shdr, - const struct llext_load_param *ldr_parm, elf_shdr_t *tgt) +static int llext_link_plt(struct llext_loader *ldr, struct llext *ext, elf_shdr_t *shdr, + const struct llext_load_param *ldr_parm, elf_shdr_t *tgt) { unsigned int sh_cnt = shdr->sh_size / shdr->sh_entsize; /* @@ -255,6 +257,7 @@ static void llext_link_plt(struct llext_loader *ldr, struct llext *ext, elf_shdr * reference point */ uint8_t *text = ext->mem[LLEXT_MEM_TEXT]; + int link_err = 0; LOG_DBG("Found %p in PLT %u size %zu cnt %u text %p", (void *)llext_section_name(ldr, ext, shdr), @@ -370,20 +373,34 @@ static void llext_link_plt(struct llext_loader *ldr, struct llext *ext, elf_shdr if (!link_addr) { LOG_WRN("PLT: cannot find idx %u name %s", j, name); - continue; + /* Will fail after reporting all missing symbols */ + if (!link_err) { + link_err = -ENOENT; + } + break; } /* Resolve the symbol */ - arch_elf_relocate_global(ldr, ext, &rela, &sym, rel_addr, link_addr); + ret = arch_elf_relocate_global(ldr, ext, &rela, &sym, rel_addr, link_addr); + if (!link_err) { + link_err = ret; + } break; case STB_LOCAL: - arch_elf_relocate_local(ldr, ext, &rela, &sym, rel_addr, ldr_parm); + ret = arch_elf_relocate_local(ldr, ext, &rela, &sym, rel_addr, ldr_parm); + if (!link_err) { + link_err = ret; + } } - LOG_DBG("symbol %s relocation @%p r-offset %#zx .text offset %#zx stb %u", - name, (void *)rel_addr, - (size_t)rela.r_offset, (size_t)ldr->sects[LLEXT_MEM_TEXT].sh_offset, stb); + if (!link_err) { + LOG_DBG("symbol %s relocation @%p r-offset %#zx .text offset %#zx stb %u", + name, (void *)rel_addr, (size_t)rela.r_offset, + (size_t)ldr->sects[LLEXT_MEM_TEXT].sh_offset, stb); + } } + + return link_err; } int llext_link(struct llext_loader *ldr, struct llext *ext, const struct llext_load_param *ldr_parm) @@ -457,7 +474,10 @@ int llext_link(struct llext_loader *ldr, struct llext *ext, const struct llext_l tgt = ext->sect_hdrs + shdr->sh_info; } - llext_link_plt(ldr, ext, shdr, ldr_parm, tgt); + ret = llext_link_plt(ldr, ext, shdr, ldr_parm, tgt); + if (ret < 0) { + return ret; + } continue; }