Re: [RFC 2/3] kbuild: modules: Build and trigger BTI scanner for arm64

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] scripts/module-bti-check: missing elf_flagdata() on relocation buffer
- [High] kbuild: module-bti-check ignores R_AARCH64_JUMP26 tail calls causing BTI panics
- [Medium] kbuild: unconditional bti_check execution breaks incremental builds
--

commit 6f17af1f0f493574af3f26ef3b98809bf3a9aca7
Author: Jeremy Linton <jeremy.linton@arm.com>

kbuild: modules: Build and trigger BTI scanner for arm64
    
Now that we have a utility which can detect and correct missing
landing pads, build it and hook it up to scan during module
finalization.

> diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
> index 01a37ec872b90..973adfa72a7d4 100644
> --- a/scripts/Makefile.modfinal
> +++ b/scripts/Makefile.modfinal
[ ... ]
> @@ -47,8 +53,12 @@ quiet_cmd_btf_ko = BTF [M] $@
>  	fi;
>  
>  # Re-generate module BTFs if either module's .ko or vmlinux changed
> -%.ko: %.o %.mod.o .module-common.o $(objtree)/scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),$(objtree)/vmlinux) FORCE
> +%.ko: %.o %.mod.o .module-common.o $(objtree)/scripts/module.lds $(bti_check_dep) $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),$(objtree)/vmlinux) FORCE
>  	+$(call if_changed,ld_ko_o)
> +ifdef CONFIG_ARM64_BTI_KERNEL
> +	+$(call cmd,bti_check)
> +endif
> +
>  ifdef CONFIG_DEBUG_INFO_BTF_MODULES
>  	+$(if $(newer-prereqs),$(call cmd,btf_ko))
>  endif

[Severity: Medium]
Will this execute the bti_check unconditionally on every incremental build?

Since the %.ko target uses FORCE, Kbuild relies on conditional macros to
prevent redundant execution. The bti_check command is not wrapped in an
if_changed guard.

Could this cause module-bti-check --fix to run on all modules every time
Kbuild is invoked, breaking the fast path for null builds?

While reviewing this, I also noticed a couple of regressions in
scripts/module-bti-check.c:

scripts/module-bti-check.c:scan_relocations() {
...
			if (!gelf_getrela(data, j, &rela))
				goto out_elferr;

			if (GELF_R_TYPE(rela.r_info) != R_AARCH64_CALL26)
				continue;
...
}

[Severity: High]
Are R_AARCH64_JUMP26 tail calls intentionally filtered out here?

If a module contains a cross-section tail call to a static function, and the
distance requires a PLT, the PLT entry will use an indirect branch to target
the function.

Since static targets don't emit their own BTI pads and the JUMP26 relocation
is bypassed here, wouldn't this indirect branch trigger a BTI exception and a
kernel panic at runtime?

scripts/module-bti-check.c:scan_relocations() {
...
				rela.r_info = GELF_R_INFO(veneer->secsym, R_AARCH64_CALL26);
				rela.r_addend = veneer->value;
				if (!gelf_update_rela(data, j, &rela))
					return elf_error(obj);

				obj->changed = true;
				continue;
...
}

[Severity: High]
Are these modified relocation entries successfully written back to the file?

gelf_update_rela() modifies the Elf_Data relocation block in memory, but the
data block is never flagged as dirty with elf_flagdata().

According to libelf semantics, won't elf_update() silently discard these
relocation changes because the buffer isn't marked dirty?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006221815.2823252-1-jeremy.linton@arm.com?part=2