Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Direct memory assignment to the PLT section bypasses text patching, causing a Data Abort when a livepatch module applies relocations to a newly loaded target module. - [High] The `aarch64_insn_set()` function incorrectly writes A64 instructions in host byte order, causing BTI veneer pages to be filled with corrupt instruction encodings on Big Endian systems. -- --- Patch [2]: [PATCH 02/12] arm64/module: Fix BTI exceptions caused by omitted landing pads in Clang 21 --- commit 74ebe0a6ff8b7ada8b741ca9247269a7b1b55fb6 Author: Josh Poimboeuf <[email protected]> arm64/module: Fix BTI exceptions caused by omitted landing pads in Clang 21 This commit fixes BTI exceptions caused by Clang 21+ omitting landing pads for static functions. It allocates a page close to the target to hold BTI veneers which receive PLT indirect branches and direct branch to the final target. > diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c > index 7afd370da9f48..4ba31e336deb6 100644 > --- a/arch/arm64/kernel/module-plts.c > +++ b/arch/arm64/kernel/module-plts.c [ ... ] > @@ -66,6 +72,180 @@ static bool plt_entries_equal(const struct plt_entry *a, [ ... ] > +static struct bti_veneer_page *bti_veneer_page_alloc(struct module *mod, > + u64 target) > +{ [ ... ] > + if (!p) { > + pr_err("%s: no address space within branch range of %pS for a > BTI veneer\n", > + mod->name, (void *)target); > + return NULL; > + } > + > + /* Don't leave unused slots executable */ > + aarch64_insn_set(p, AARCH64_BREAK_FAULT, PAGE_SIZE); [Severity: High] This is a pre-existing issue with aarch64_insn_set(), but does this new usage write the AARCH64_BREAK_FAULT instruction in the wrong byte order on Big Endian systems? Looking at aarch64_insn_set(), it passes the host-endian integer to memset32(), which performs native 32-bit stores without cpu_to_le32() conversion: arch/arm64/kernel/patching.c:aarch64_insn_set() { ... return __text_poke(text_poke_memset, dst, &insn, len); } Since ARMv8 instruction fetches are architecturally always Little Endian, wouldn't the CPU fetch a byte-reversed, corrupt instruction (reading 0x000020d4) on Big Endian systems, resulting in a fatal Unknown instruction exception rather than a BRK exception if an unused slot is executed? > + > + page = kzalloc_obj(*page, GFP_KERNEL); > + if (!page) { [ ... ] > @@ -77,6 +257,12 @@ u64 module_emit_plt_entry(struct module *mod, Elf64_Shdr > *sechdrs, > void *loc, const Elf64_Rela *rela, > Elf64_Sym *sym) > { [ ... ] > u64 val = sym->st_value + rela->r_addend; > > + if (!plt_target_has_landing_pad(val)) { > + val = module_emit_bti_veneer(mod, val); > + if (!val) > + return 0; > + } > + > if (is_forbidden_offset_for_adrp(&plt[i].adrp)) > i++; > > plt[i] = get_plt_entry(val, &plt[i]); [Severity: High] This isn't a bug introduced by this patch, but can this direct memory assignment cause a Data Abort kernel panic when a livepatch module applies relocations to a newly loaded target module? If a livepatch module is already loaded and active, its sections (including the PLT) are marked ROX. When a new target module loads, klp_module_coming() calls apply_relocate_add() to apply the patch module's relocations for this new target. If an out-of-range jump requires a new PLT entry, it seems this direct assignment to the ROX plt memory array bypasses text patching mechanisms like patch_map() or aarch64_insn_copy(). -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
