* [PATCH v2 1/2] arch: arm64: Fail module loading if dynamic SCS patching fails
2025-09-22 13:04 [PATCH v2 0/2] arm64: modules: Reject loading of malformed modules Adrian Barnaś
@ 2025-09-22 13:04 ` Adrian Barnaś
2025-09-22 13:04 ` [PATCH v2 2/2] arch: arm64: Reject modules with internal alternative callbacks Adrian Barnaś
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Adrian Barnaś @ 2025-09-22 13:04 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: Adrian Barnaś, Catalin Marinas, Will Deacon, Ard Biesheuvel,
Dylan Hatch, Mark Rutland
Disallow a module to load if SCS dynamic patching fails for its code. For
module loading, instead of running a dry-run to check for patching errors,
try to run patching in the first run and propagate any errors so module
loading will fail.
Signed-off-by: Adrian Barnaś <abarnas@google.com>
---
arch/arm64/include/asm/scs.h | 2 +-
arch/arm64/kernel/module.c | 12 ++++++++++--
arch/arm64/kernel/pi/map_kernel.c | 2 +-
arch/arm64/kernel/pi/patch-scs.c | 10 ++++++----
arch/arm64/kernel/pi/pi.h | 2 +-
5 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/include/asm/scs.h b/arch/arm64/include/asm/scs.h
index a76f9b387a26..c59f6324f2bb 100644
--- a/arch/arm64/include/asm/scs.h
+++ b/arch/arm64/include/asm/scs.h
@@ -53,7 +53,7 @@ enum {
EDYNSCS_INVALID_CFA_OPCODE = 4,
};
-int __pi_scs_patch(const u8 eh_frame[], int size);
+int __pi_scs_patch(const u8 eh_frame[], int size, bool skip_dry_run);
#endif /* __ASSEMBLY __ */
diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
index 40148d2725ce..d32ab7dd86a8 100644
--- a/arch/arm64/kernel/module.c
+++ b/arch/arm64/kernel/module.c
@@ -484,10 +484,18 @@ int module_finalize(const Elf_Ehdr *hdr,
if (scs_is_dynamic()) {
s = find_section(hdr, sechdrs, ".init.eh_frame");
if (s) {
- ret = __pi_scs_patch((void *)s->sh_addr, s->sh_size);
- if (ret)
+ /*
+ * Because we can reject modules that are malformed
+ * so SCS patching fails, skip dry run and try to patch
+ * it in place. If patching fails, the module would not
+ * be loaded anyway.
+ */
+ ret = __pi_scs_patch((void *)s->sh_addr, s->sh_size, true);
+ if (ret) {
pr_err("module %s: error occurred during dynamic SCS patching (%d)\n",
me->name, ret);
+ return -ENOEXEC;
+ }
}
}
diff --git a/arch/arm64/kernel/pi/map_kernel.c b/arch/arm64/kernel/pi/map_kernel.c
index 0f4bd7771859..7187eda9e8a5 100644
--- a/arch/arm64/kernel/pi/map_kernel.c
+++ b/arch/arm64/kernel/pi/map_kernel.c
@@ -98,7 +98,7 @@ static void __init map_kernel(u64 kaslr_offset, u64 va_offset, int root_level)
if (enable_scs) {
scs_patch(__eh_frame_start + va_offset,
- __eh_frame_end - __eh_frame_start);
+ __eh_frame_end - __eh_frame_start, false);
asm("ic ialluis");
dynamic_scs_is_enabled = true;
diff --git a/arch/arm64/kernel/pi/patch-scs.c b/arch/arm64/kernel/pi/patch-scs.c
index 55d0cd64ef71..bbe7d30ed12b 100644
--- a/arch/arm64/kernel/pi/patch-scs.c
+++ b/arch/arm64/kernel/pi/patch-scs.c
@@ -225,7 +225,7 @@ static int scs_handle_fde_frame(const struct eh_frame *frame,
return 0;
}
-int scs_patch(const u8 eh_frame[], int size)
+int scs_patch(const u8 eh_frame[], int size, bool skip_dry_run)
{
int code_alignment_factor = 1;
bool fde_use_sdata8 = false;
@@ -277,11 +277,13 @@ int scs_patch(const u8 eh_frame[], int size)
}
} else {
ret = scs_handle_fde_frame(frame, code_alignment_factor,
- fde_use_sdata8, true);
+ fde_use_sdata8, !skip_dry_run);
if (ret)
return ret;
- scs_handle_fde_frame(frame, code_alignment_factor,
- fde_use_sdata8, false);
+
+ if (!skip_dry_run)
+ scs_handle_fde_frame(frame, code_alignment_factor,
+ fde_use_sdata8, false);
}
p += sizeof(frame->size) + frame->size;
diff --git a/arch/arm64/kernel/pi/pi.h b/arch/arm64/kernel/pi/pi.h
index 46cafee7829f..220da972cce1 100644
--- a/arch/arm64/kernel/pi/pi.h
+++ b/arch/arm64/kernel/pi/pi.h
@@ -27,7 +27,7 @@ extern pgd_t init_pg_dir[], init_pg_end[];
void init_feature_override(u64 boot_status, const void *fdt, int chosen);
u64 kaslr_early_init(void *fdt, int chosen);
void relocate_kernel(u64 offset);
-int scs_patch(const u8 eh_frame[], int size);
+int scs_patch(const u8 eh_frame[], int size, bool skip_dry_run);
void map_range(u64 *pgd, u64 start, u64 end, u64 pa, pgprot_t prot,
int level, pte_t *tbl, bool may_use_cont, u64 va_offset);
--
2.51.0.534.gc79095c0ca-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/2] arch: arm64: Reject modules with internal alternative callbacks
2025-09-22 13:04 [PATCH v2 0/2] arm64: modules: Reject loading of malformed modules Adrian Barnaś
2025-09-22 13:04 ` [PATCH v2 1/2] arch: arm64: Fail module loading if dynamic SCS patching fails Adrian Barnaś
@ 2025-09-22 13:04 ` Adrian Barnaś
2025-11-07 9:57 ` Mark Rutland
2025-09-29 16:10 ` [PATCH v2 0/2] arm64: modules: Reject loading of malformed modules Ard Biesheuvel
2025-11-07 15:53 ` Will Deacon
3 siblings, 1 reply; 6+ messages in thread
From: Adrian Barnaś @ 2025-09-22 13:04 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: Adrian Barnaś, Catalin Marinas, Will Deacon, Ard Biesheuvel,
Dylan Hatch, Mark Rutland, Fanqin Cui
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(-)
diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
index 00d97b8a757f..51746005239b 100644
--- a/arch/arm64/include/asm/alternative.h
+++ b/arch/arm64/include/asm/alternative.h
@@ -26,9 +26,12 @@ void __init apply_alternatives_all(void);
bool alternative_is_applied(u16 cpucap);
#ifdef CONFIG_MODULES
-void apply_alternatives_module(void *start, size_t length);
+int apply_alternatives_module(void *start, size_t length);
#else
-static inline void apply_alternatives_module(void *start, size_t length) { }
+static inline int apply_alternatives_module(void *start, size_t length)
+{
+ return 0;
+}
#endif
void alt_cb_patch_nops(struct alt_instr *alt, __le32 *origptr,
diff --git a/arch/arm64/kernel/alternative.c b/arch/arm64/kernel/alternative.c
index 8ff6610af496..11893a0360ad 100644
--- a/arch/arm64/kernel/alternative.c
+++ b/arch/arm64/kernel/alternative.c
@@ -139,9 +139,9 @@ static noinstr void clean_dcache_range_nopatch(u64 start, u64 end)
} while (cur += d_size, cur < end);
}
-static void __apply_alternatives(const struct alt_region *region,
- bool is_module,
- unsigned long *cpucap_mask)
+static int __apply_alternatives(const struct alt_region *region,
+ bool is_module,
+ unsigned long *cpucap_mask)
{
struct alt_instr *alt;
__le32 *origptr, *updptr;
@@ -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;
+ }
alt_cb(alt, origptr, updptr, nr_inst);
@@ -193,6 +196,8 @@ static void __apply_alternatives(const struct alt_region *region,
bitmap_and(applied_alternatives, applied_alternatives,
system_cpucaps, ARM64_NCAPS);
}
+
+ return 0;
}
static void __init apply_alternatives_vdso(void)
@@ -277,7 +282,7 @@ void __init apply_boot_alternatives(void)
}
#ifdef CONFIG_MODULES
-void apply_alternatives_module(void *start, size_t length)
+int apply_alternatives_module(void *start, size_t length)
{
struct alt_region region = {
.begin = start,
@@ -287,7 +292,7 @@ void apply_alternatives_module(void *start, size_t length)
bitmap_fill(all_capabilities, ARM64_NCAPS);
- __apply_alternatives(®ion, true, &all_capabilities[0]);
+ return __apply_alternatives(®ion, true, &all_capabilities[0]);
}
#endif
diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
index d32ab7dd86a8..6e5b488a219e 100644
--- a/arch/arm64/kernel/module.c
+++ b/arch/arm64/kernel/module.c
@@ -478,8 +478,13 @@ int module_finalize(const Elf_Ehdr *hdr,
int ret;
s = find_section(hdr, sechdrs, ".altinstructions");
- if (s)
- apply_alternatives_module((void *)s->sh_addr, s->sh_size);
+ if (s) {
+ ret = apply_alternatives_module((void *)s->sh_addr, s->sh_size);
+ if (ret < 0) {
+ pr_err("module %s: error occurred when applying alternatives\n", me->name);
+ return ret;
+ }
+ }
if (scs_is_dynamic()) {
s = find_section(hdr, sechdrs, ".init.eh_frame");
--
2.51.0.534.gc79095c0ca-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/2] arch: arm64: Reject modules with internal alternative callbacks
2025-09-22 13:04 ` [PATCH v2 2/2] arch: arm64: Reject modules with internal alternative callbacks Adrian Barnaś
@ 2025-11-07 9:57 ` Mark Rutland
0 siblings, 0 replies; 6+ messages in thread
From: Mark Rutland @ 2025-11-07 9:57 UTC (permalink / raw)
To: Adrian Barnaś, Will Deacon
Cc: linux-arm-kernel, linux-kernel, Catalin Marinas, Ard Biesheuvel,
Dylan Hatch, Fanqin Cui
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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/2] arm64: modules: Reject loading of malformed modules
2025-09-22 13:04 [PATCH v2 0/2] arm64: modules: Reject loading of malformed modules Adrian Barnaś
2025-09-22 13:04 ` [PATCH v2 1/2] arch: arm64: Fail module loading if dynamic SCS patching fails Adrian Barnaś
2025-09-22 13:04 ` [PATCH v2 2/2] arch: arm64: Reject modules with internal alternative callbacks Adrian Barnaś
@ 2025-09-29 16:10 ` Ard Biesheuvel
2025-11-07 15:53 ` Will Deacon
3 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2025-09-29 16:10 UTC (permalink / raw)
To: Adrian Barnaś
Cc: linux-arm-kernel, linux-kernel, Catalin Marinas, Will Deacon,
Dylan Hatch, Mark Rutland
On Mon, 22 Sept 2025 at 15:04, Adrian Barnaś <abarnas@google.com> wrote:
>
> Hi all,
>
> Here is version two of the patches I previously posted here:
>
> v1: https://lore.kernel.org/all/20250919122321.946462-1-abarnas@google.com/
>
> Changes:
> * Renamed the parameter `is_module` to `skip_dry_run` in scs_patch()
> * Moved comments to module_finalize() and improve justification
> * Instead of rejecting all modules with callback, reject those with cb
> pointing outside core kernel text
> * Replace -EPERM to -ENOEXEC when rejecting modules with incorrect cb
> * Fix missing return in apply_alternatives_module() placeholder
>
> Best regards
> Adrian
>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Dylan Hatch <dylanbhatch@google.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
>
> Adrian Barnaś (2):
> arch: arm64: Fail module loading if dynamic SCS patching fails
> arch: arm64: Reject modules with internal alternative callbacks
>
Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/2] arm64: modules: Reject loading of malformed modules
2025-09-22 13:04 [PATCH v2 0/2] arm64: modules: Reject loading of malformed modules Adrian Barnaś
` (2 preceding siblings ...)
2025-09-29 16:10 ` [PATCH v2 0/2] arm64: modules: Reject loading of malformed modules Ard Biesheuvel
@ 2025-11-07 15:53 ` Will Deacon
3 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2025-11-07 15:53 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, Adrian Barnaś
Cc: catalin.marinas, kernel-team, Will Deacon, Ard Biesheuvel,
Dylan Hatch, Mark Rutland
On Mon, 22 Sep 2025 13:04:25 +0000, Adrian Barnaś wrote:
> Here is version two of the patches I previously posted here:
>
> v1: https://lore.kernel.org/all/20250919122321.946462-1-abarnas@google.com/
>
> Changes:
> * Renamed the parameter `is_module` to `skip_dry_run` in scs_patch()
> * Moved comments to module_finalize() and improve justification
> * Instead of rejecting all modules with callback, reject those with cb
> pointing outside core kernel text
> * Replace -EPERM to -ENOEXEC when rejecting modules with incorrect cb
> * Fix missing return in apply_alternatives_module() placeholder
>
> [...]
Applied to arm64 (for-next/fixes), thanks!
[1/2] arm64: Fail module loading if dynamic SCS patching fails
https://git.kernel.org/arm64/c/6d4a0fbd34a4
[2/2] arm64: Reject modules with internal alternative callbacks
https://git.kernel.org/arm64/c/8e8ae788964a
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
^ permalink raw reply [flat|nested] 6+ messages in thread