Re: [PATCH v2 2/2] arch: arm64: Reject modules with internal alternative callbacks

[Mark Rutland <mark.rutland@arm.com>]

On Mon, Sep 22, 2025 at 01:04:27PM +0000, Adrian Barnaś wrote:
> During module loading, check if a callback function used by the
> alternatives specified in the '.altinstruction' ELF section (if present)
> is located in core kernel .text. If not fail module loading before
> callback is called.
> 
> Reported-by: Fanqin Cui <cuifq1@chinatelecom.cn>
> Closes: https://lore.kernel.org/all/20250807072700.348514-1-fanqincui@163.com/
> Signed-off-by: Adrian Barnaś <abarnas@google.com>
> ---
>  arch/arm64/include/asm/alternative.h |  7 +++++--
>  arch/arm64/kernel/alternative.c      | 19 ++++++++++++-------
>  arch/arm64/kernel/module.c           |  9 +++++++--
>  3 files changed, 24 insertions(+), 11 deletions(-)

[...]

> @@ -166,10 +166,13 @@ static void __apply_alternatives(const struct alt_region *region,
>  		updptr = is_module ? origptr : lm_alias(origptr);
>  		nr_inst = alt->orig_len / AARCH64_INSN_SIZE;
>  
> -		if (ALT_HAS_CB(alt))
> +		if (ALT_HAS_CB(alt)) {
>  			alt_cb  = ALT_REPL_PTR(alt);
> -		else
> +			if (!core_kernel_text((unsigned long)alt_cb))
> +				return -ENOEXEC;
> +		} else {
>  			alt_cb = patch_alternative;
> +		}

There's an existing noinstr safety issue there that this makes a bit
worse.

The core_kernel_text() function is instrumentable, and so for
(non-module) alternatives, calling that in the middle of patching isn't
safe (as it could lead to calling arbitrary C code mid-patching).

That said, __apply_alternatives() aren't marked as noinstr, and cleaning
that up properly is going to require some major rework. I don't think we
want to block this patch on that.

I think we can bodge around the worst of that for now with:

	if (is_module && !core_kernel_text((unsigned long)alt_cb))
		return -ENOEXEC;

... which'll avoid calling out to other instrumentable code during the
patching sequence, and minimize the risk of that blowing up during boot.

I'll see about prioritizing a follow-up to fix the extant issues more
thoroughly.

Will, are you happy to add the 'is module &&' part to the condition?

Mark.