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

Mark Rutland
2025-11-07T09:58:00Z

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.