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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aQ3ClNpcT-YBlvnG@J2N7QTR9R3>
Date: Fri, 7 Nov 2025 09:57:40 +0000
From: Mark Rutland <mark.rutland@....com>
To: Adrian Barnaś <abarnas@...gle.com>,
	Will Deacon <will@...nel.org>
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Catalin Marinas <catalin.marinas@....com>,
	Ard Biesheuvel <ardb@...nel.org>,
	Dylan Hatch <dylanbhatch@...gle.com>,
	Fanqin Cui <cuifq1@...natelecom.cn>
Subject: Re: [PATCH v2 2/2] arch: arm64: Reject modules with internal
 alternative callbacks

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@...natelecom.cn>
> Closes: https://lore.kernel.org/all/20250807072700.348514-1-fanqincui@163.com/
> Signed-off-by: Adrian Barnaś <abarnas@...gle.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.