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

[sashiko-bot]

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