Re: [PATCH 02/12] arm64/module: Fix BTI exceptions caused by omitted landing pads in Clang 21

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 [off-list ref]
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.

[ ... ]

[ ... ]

+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) {

[ ... ]

[ ... ]

 	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/cover.1786768375.git.jpoimboe@kernel.org?part=2