During module loading, check if there is a callback function used by the
alternatives specified in the '.altinstruction' ELF section and block
loading the module if such a function is present.
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 | 4 ++--
arch/arm64/kernel/alternative.c | 15 ++++++++++-----
arch/arm64/kernel/module.c | 8 ++++++--
3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
index 00d97b8a757f..6eab2517c809 100644
--- a/arch/arm64/include/asm/alternative.h
+++ b/arch/arm64/include/asm/alternative.h
@@ -26,9 +26,9 @@ 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) { }
#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..69fae94c843a 100644
--- a/arch/arm64/kernel/alternative.c
+++ b/arch/arm64/kernel/alternative.c
@@ -139,15 +139,18 @@ 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;
alternative_cb_t alt_cb;
for (alt = region->begin; alt < region->end; alt++) {
+ if (ALT_HAS_CB(alt) && is_module)
+ return -EPERM;
+
int nr_inst;
int cap = ALT_CAP(alt);
@@ -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 5d6d228c6156..aeefc50229e3 100644
--- a/arch/arm64/kernel/module.c
+++ b/arch/arm64/kernel/module.c
@@ -478,8 +478,12 @@ 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
Hi Adrian, kernel test robot noticed the following build warnings: [auto build test WARNING on arm/for-next] [also build test WARNING on arm/fixes kvmarm/next soc/for-next linus/master v6.17-rc6 next-20250919] [cannot apply to arm64/for-next/core] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Adrian-Barna/arch-arm64-Fail-module-loading-if-dynamic-SCS-patching-fails/20250919-202643 base: git://git.armlinux.org.uk/~rmk/linux-arm.git for-next patch link: https://lore.kernel.org/r/20250919122321.946462-3-abarnas%40google.com patch subject: [PATCH 2/2] arch: arm64: Reject modules with internal alternative callbacks config: arm64-allnoconfig (https://download.01.org/0day-ci/archive/20250920/202509200821.Qy7LYrJP-lkp@intel.com/config) compiler: aarch64-linux-gcc (GCC) 15.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250920/202509200821.Qy7LYrJP-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202509200821.Qy7LYrJP-lkp@intel.com/ All warnings (new ones prefixed by >>): In file included from arch/arm64/include/asm/sysreg.h:1111, from arch/arm64/include/asm/cputype.h:250, from arch/arm64/include/asm/cache.h:43, from include/vdso/cache.h:5, from include/linux/cache.h:6, from include/linux/slab.h:15, from include/linux/resource_ext.h:11, from include/linux/acpi.h:13, from include/acpi/apei.h:9, from include/acpi/ghes.h:5, from include/linux/arm_sdei.h:8, from arch/arm64/kernel/asm-offsets.c:10: arch/arm64/include/asm/alternative.h: In function 'apply_alternatives_module': >> arch/arm64/include/asm/alternative.h:31:1: warning: no return statement in function returning non-void [-Wreturn-type] 31 | static inline int apply_alternatives_module(void *start, size_t length) { } | ^~~~~~ -- In file included from arch/arm64/include/asm/sysreg.h:1111, from arch/arm64/include/asm/memory.h:223, from arch/arm64/include/asm/thread_info.h:17, from include/linux/thread_info.h:60, from arch/arm64/include/asm/preempt.h:6, from include/linux/preempt.h:79, from include/linux/spinlock.h:56, from include/linux/wait.h:9, from include/linux/wait_bit.h:8, from include/linux/fs.h:7, from arch/arm64/kernel/sys.c:11: arch/arm64/include/asm/alternative.h: In function 'apply_alternatives_module': >> arch/arm64/include/asm/alternative.h:31:1: warning: no return statement in function returning non-void [-Wreturn-type] 31 | static inline int apply_alternatives_module(void *start, size_t length) { } | ^~~~~~ ./arch/arm64/include/generated/asm/syscall_table_64.h: At top level: arch/arm64/kernel/sys.c:58:40: warning: initialized field overwritten [-Woverride-init] 58 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym, | ^~~~~~~~ arch/arm64/kernel/sys.c:51:52: note: in expansion of macro '__SYSCALL' 51 | #define __SYSCALL_WITH_COMPAT(nr, native, compat) __SYSCALL(nr, native) | ^~~~~~~~~ ./arch/arm64/include/generated/asm/syscall_table_64.h:1:1: note: in expansion of macro '__SYSCALL_WITH_COMPAT' 1 | __SYSCALL_WITH_COMPAT(0, sys_io_setup, compat_sys_io_setup) | ^~~~~~~~~~~~~~~~~~~~~ arch/arm64/kernel/sys.c:58:40: note: (near initialization for 'sys_call_table[0]') 58 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym, | ^~~~~~~~ arch/arm64/kernel/sys.c:51:52: note: in expansion of macro '__SYSCALL' 51 | #define __SYSCALL_WITH_COMPAT(nr, native, compat) __SYSCALL(nr, native) | ^~~~~~~~~ ./arch/arm64/include/generated/asm/syscall_table_64.h:1:1: note: in expansion of macro '__SYSCALL_WITH_COMPAT' 1 | __SYSCALL_WITH_COMPAT(0, sys_io_setup, compat_sys_io_setup) | ^~~~~~~~~~~~~~~~~~~~~ arch/arm64/kernel/sys.c:58:40: warning: initialized field overwritten [-Woverride-init] 58 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym, | ^~~~~~~~ ./arch/arm64/include/generated/asm/syscall_table_64.h:2:1: note: in expansion of macro '__SYSCALL' 2 | __SYSCALL(1, sys_io_destroy) | ^~~~~~~~~ arch/arm64/kernel/sys.c:58:40: note: (near initialization for 'sys_call_table[1]') 58 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym, | ^~~~~~~~ ./arch/arm64/include/generated/asm/syscall_table_64.h:2:1: note: in expansion of macro '__SYSCALL' 2 | __SYSCALL(1, sys_io_destroy) | ^~~~~~~~~ arch/arm64/kernel/sys.c:58:40: warning: initialized field overwritten [-Woverride-init] 58 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym, | ^~~~~~~~ arch/arm64/kernel/sys.c:51:52: note: in expansion of macro '__SYSCALL' 51 | #define __SYSCALL_WITH_COMPAT(nr, native, compat) __SYSCALL(nr, native) | ^~~~~~~~~ ./arch/arm64/include/generated/asm/syscall_table_64.h:3:1: note: in expansion of macro '__SYSCALL_WITH_COMPAT' 3 | __SYSCALL_WITH_COMPAT(2, sys_io_submit, compat_sys_io_submit) | ^~~~~~~~~~~~~~~~~~~~~ arch/arm64/kernel/sys.c:58:40: note: (near initialization for 'sys_call_table[2]') 58 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym, | ^~~~~~~~ arch/arm64/kernel/sys.c:51:52: note: in expansion of macro '__SYSCALL' 51 | #define __SYSCALL_WITH_COMPAT(nr, native, compat) __SYSCALL(nr, native) | ^~~~~~~~~ ./arch/arm64/include/generated/asm/syscall_table_64.h:3:1: note: in expansion of macro '__SYSCALL_WITH_COMPAT' 3 | __SYSCALL_WITH_COMPAT(2, sys_io_submit, compat_sys_io_submit) | ^~~~~~~~~~~~~~~~~~~~~ arch/arm64/kernel/sys.c:58:40: warning: initialized field overwritten [-Woverride-init] 58 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym, | ^~~~~~~~ ./arch/arm64/include/generated/asm/syscall_table_64.h:4:1: note: in expansion of macro '__SYSCALL' 4 | __SYSCALL(3, sys_io_cancel) | ^~~~~~~~~ arch/arm64/kernel/sys.c:58:40: note: (near initialization for 'sys_call_table[3]') 58 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym, | ^~~~~~~~ ./arch/arm64/include/generated/asm/syscall_table_64.h:4:1: note: in expansion of macro '__SYSCALL' 4 | __SYSCALL(3, sys_io_cancel) | ^~~~~~~~~ arch/arm64/kernel/sys.c:58:40: warning: initialized field overwritten [-Woverride-init] 58 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym, | ^~~~~~~~ ./arch/arm64/include/generated/asm/syscall_table_64.h:5:1: note: in expansion of macro '__SYSCALL' 5 | __SYSCALL(4, sys_io_getevents) | ^~~~~~~~~ arch/arm64/kernel/sys.c:58:40: note: (near initialization for 'sys_call_table[4]') 58 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym, | ^~~~~~~~ ./arch/arm64/include/generated/asm/syscall_table_64.h:5:1: note: in expansion of macro '__SYSCALL' 5 | __SYSCALL(4, sys_io_getevents) | ^~~~~~~~~ arch/arm64/kernel/sys.c:58:40: warning: initialized field overwritten [-Woverride-init] 58 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym, | ^~~~~~~~ ./arch/arm64/include/generated/asm/syscall_table_64.h:6:1: note: in expansion of macro '__SYSCALL' 6 | __SYSCALL(5, sys_setxattr) | ^~~~~~~~~ arch/arm64/kernel/sys.c:58:40: note: (near initialization for 'sys_call_table[5]') 58 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym, | ^~~~~~~~ ./arch/arm64/include/generated/asm/syscall_table_64.h:6:1: note: in expansion of macro '__SYSCALL' 6 | __SYSCALL(5, sys_setxattr) | ^~~~~~~~~ arch/arm64/kernel/sys.c:58:40: warning: initialized field overwritten [-Woverride-init] 58 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym, | ^~~~~~~~ ./arch/arm64/include/generated/asm/syscall_table_64.h:7:1: note: in expansion of macro '__SYSCALL' 7 | __SYSCALL(6, sys_lsetxattr) | ^~~~~~~~~ arch/arm64/kernel/sys.c:58:40: note: (near initialization for 'sys_call_table[6]') 58 | #define __SYSCALL(nr, sym) [nr] = __arm64_##sym, | ^~~~~~~~ ./arch/arm64/include/generated/asm/syscall_table_64.h:7:1: note: in expansion of macro '__SYSCALL' 7 | __SYSCALL(6, sys_lsetxattr) | ^~~~~~~~~ arch/arm64/kernel/sys.c:58:40: warning: initialized field overwritten [-Woverride-init] -- In file included from arch/arm64/include/asm/sysreg.h:1111, from arch/arm64/include/asm/cputype.h:250, from arch/arm64/include/asm/cache.h:43, from include/vdso/cache.h:5, from include/linux/cache.h:6, from arch/arm64/include/asm/processor.h:29, from include/linux/sched.h:13, from include/linux/context_tracking.h:5, from arch/arm64/kernel/traps.c:10: arch/arm64/include/asm/alternative.h: In function 'apply_alternatives_module': >> arch/arm64/include/asm/alternative.h:31:1: warning: no return statement in function returning non-void [-Wreturn-type] 31 | static inline int apply_alternatives_module(void *start, size_t length) { } | ^~~~~~ arch/arm64/kernel/traps.c: At top level: arch/arm64/kernel/traps.c:831:43: warning: initialized field overwritten [-Woverride-init] 831 | [ESR_ELx_EC_UNKNOWN] = "Unknown/Uncategorized", | ^~~~~~~~~~~~~~~~~~~~~~~ arch/arm64/kernel/traps.c:831:43: note: (near initialization for 'esr_class_str[0]') arch/arm64/kernel/traps.c:832:43: warning: initialized field overwritten [-Woverride-init] 832 | [ESR_ELx_EC_WFx] = "WFI/WFE", | ^~~~~~~~~ arch/arm64/kernel/traps.c:832:43: note: (near initialization for 'esr_class_str[1]') arch/arm64/kernel/traps.c:833:43: warning: initialized field overwritten [-Woverride-init] 833 | [ESR_ELx_EC_CP15_32] = "CP15 MCR/MRC", | ^~~~~~~~~~~~~~ arch/arm64/kernel/traps.c:833:43: note: (near initialization for 'esr_class_str[3]') arch/arm64/kernel/traps.c:834:43: warning: initialized field overwritten [-Woverride-init] 834 | [ESR_ELx_EC_CP15_64] = "CP15 MCRR/MRRC", | ^~~~~~~~~~~~~~~~ arch/arm64/kernel/traps.c:834:43: note: (near initialization for 'esr_class_str[4]') arch/arm64/kernel/traps.c:835:43: warning: initialized field overwritten [-Woverride-init] 835 | [ESR_ELx_EC_CP14_MR] = "CP14 MCR/MRC", | ^~~~~~~~~~~~~~ arch/arm64/kernel/traps.c:835:43: note: (near initialization for 'esr_class_str[5]') arch/arm64/kernel/traps.c:836:43: warning: initialized field overwritten [-Woverride-init] 836 | [ESR_ELx_EC_CP14_LS] = "CP14 LDC/STC", | ^~~~~~~~~~~~~~ arch/arm64/kernel/traps.c:836:43: note: (near initialization for 'esr_class_str[6]') arch/arm64/kernel/traps.c:837:43: warning: initialized field overwritten [-Woverride-init] 837 | [ESR_ELx_EC_FP_ASIMD] = "ASIMD", | ^~~~~~~ arch/arm64/kernel/traps.c:837:43: note: (near initialization for 'esr_class_str[7]') arch/arm64/kernel/traps.c:838:43: warning: initialized field overwritten [-Woverride-init] 838 | [ESR_ELx_EC_CP10_ID] = "CP10 MRC/VMRS", | ^~~~~~~~~~~~~~~ arch/arm64/kernel/traps.c:838:43: note: (near initialization for 'esr_class_str[8]') arch/arm64/kernel/traps.c:839:43: warning: initialized field overwritten [-Woverride-init] 839 | [ESR_ELx_EC_PAC] = "PAC", | ^~~~~ arch/arm64/kernel/traps.c:839:43: note: (near initialization for 'esr_class_str[9]') arch/arm64/kernel/traps.c:840:43: warning: initialized field overwritten [-Woverride-init] 840 | [ESR_ELx_EC_CP14_64] = "CP14 MCRR/MRRC", | ^~~~~~~~~~~~~~~~ arch/arm64/kernel/traps.c:840:43: note: (near initialization for 'esr_class_str[12]') arch/arm64/kernel/traps.c:841:43: warning: initialized field overwritten [-Woverride-init] 841 | [ESR_ELx_EC_BTI] = "BTI", | ^~~~~ arch/arm64/kernel/traps.c:841:43: note: (near initialization for 'esr_class_str[13]') arch/arm64/kernel/traps.c:842:43: warning: initialized field overwritten [-Woverride-init] 842 | [ESR_ELx_EC_ILL] = "PSTATE.IL", | ^~~~~~~~~~~ arch/arm64/kernel/traps.c:842:43: note: (near initialization for 'esr_class_str[14]') arch/arm64/kernel/traps.c:843:43: warning: initialized field overwritten [-Woverride-init] 843 | [ESR_ELx_EC_SVC32] = "SVC (AArch32)", | ^~~~~~~~~~~~~~~ arch/arm64/kernel/traps.c:843:43: note: (near initialization for 'esr_class_str[17]') arch/arm64/kernel/traps.c:844:43: warning: initialized field overwritten [-Woverride-init] 844 | [ESR_ELx_EC_HVC32] = "HVC (AArch32)", | ^~~~~~~~~~~~~~~ arch/arm64/kernel/traps.c:844:43: note: (near initialization for 'esr_class_str[18]') arch/arm64/kernel/traps.c:845:43: warning: initialized field overwritten [-Woverride-init] 845 | [ESR_ELx_EC_SMC32] = "SMC (AArch32)", | ^~~~~~~~~~~~~~~ arch/arm64/kernel/traps.c:845:43: note: (near initialization for 'esr_class_str[19]') arch/arm64/kernel/traps.c:846:43: warning: initialized field overwritten [-Woverride-init] 846 | [ESR_ELx_EC_SVC64] = "SVC (AArch64)", | ^~~~~~~~~~~~~~~ arch/arm64/kernel/traps.c:846:43: note: (near initialization for 'esr_class_str[21]') arch/arm64/kernel/traps.c:847:43: warning: initialized field overwritten [-Woverride-init] 847 | [ESR_ELx_EC_HVC64] = "HVC (AArch64)", | ^~~~~~~~~~~~~~~ arch/arm64/kernel/traps.c:847:43: note: (near initialization for 'esr_class_str[22]') arch/arm64/kernel/traps.c:848:43: warning: initialized field overwritten [-Woverride-init] 848 | [ESR_ELx_EC_SMC64] = "SMC (AArch64)", | ^~~~~~~~~~~~~~~ arch/arm64/kernel/traps.c:848:43: note: (near initialization for 'esr_class_str[23]') arch/arm64/kernel/traps.c:849:43: warning: initialized field overwritten [-Woverride-init] 849 | [ESR_ELx_EC_SYS64] = "MSR/MRS (AArch64)", | ^~~~~~~~~~~~~~~~~~~ arch/arm64/kernel/traps.c:849:43: note: (near initialization for 'esr_class_str[24]') arch/arm64/kernel/traps.c:850:43: warning: initialized field overwritten [-Woverride-init] 850 | [ESR_ELx_EC_SVE] = "SVE", | ^~~~~ arch/arm64/kernel/traps.c:850:43: note: (near initialization for 'esr_class_str[25]') arch/arm64/kernel/traps.c:851:43: warning: initialized field overwritten [-Woverride-init] 851 | [ESR_ELx_EC_ERET] = "ERET/ERETAA/ERETAB", | ^~~~~~~~~~~~~~~~~~~~ arch/arm64/kernel/traps.c:851:43: note: (near initialization for 'esr_class_str[26]') arch/arm64/kernel/traps.c:852:43: warning: initialized field overwritten [-Woverride-init] 852 | [ESR_ELx_EC_FPAC] = "FPAC", | ^~~~~~ arch/arm64/kernel/traps.c:852:43: note: (near initialization for 'esr_class_str[28]') arch/arm64/kernel/traps.c:853:43: warning: initialized field overwritten [-Woverride-init] 853 | [ESR_ELx_EC_SME] = "SME", | ^~~~~ arch/arm64/kernel/traps.c:853:43: note: (near initialization for 'esr_class_str[29]') arch/arm64/kernel/traps.c:854:43: warning: initialized field overwritten [-Woverride-init] 854 | [ESR_ELx_EC_IMP_DEF] = "EL3 IMP DEF", | ^~~~~~~~~~~~~ arch/arm64/kernel/traps.c:854:43: note: (near initialization for 'esr_class_str[31]') arch/arm64/kernel/traps.c:855:43: warning: initialized field overwritten [-Woverride-init] -- In file included from arch/arm64/include/asm/sysreg.h:1111, from arch/arm64/include/asm/cputype.h:250, from arch/arm64/include/asm/cache.h:43, from include/vdso/cache.h:5, from include/linux/cache.h:6, from include/linux/slab.h:15, from include/linux/resource_ext.h:11, from include/linux/acpi.h:13, from include/acpi/apei.h:9, from include/acpi/ghes.h:5, from include/linux/arm_sdei.h:8, from arch/arm64/kernel/asm-offsets.c:10: arch/arm64/include/asm/alternative.h: In function 'apply_alternatives_module': >> arch/arm64/include/asm/alternative.h:31:1: warning: no return statement in function returning non-void [-Wreturn-type] 31 | static inline int apply_alternatives_module(void *start, size_t length) { } | ^~~~~~ In file included from arch/arm64/include/asm/vdso/gettimeofday.h:10, from include/vdso/datapage.h:197, from lib/vdso/gettimeofday.c:5, from <command-line>: arch/arm64/include/asm/alternative.h: In function 'apply_alternatives_module': >> arch/arm64/include/asm/alternative.h:31:1: warning: no return statement in function returning non-void [-Wreturn-type] 31 | static inline int apply_alternatives_module(void *start, size_t length) { } | ^~~~~~ In file included from arch/arm64/include/asm/vdso/gettimeofday.h:10, from include/vdso/datapage.h:197, from lib/vdso/getrandom.c:8, from <command-line>: arch/arm64/include/asm/alternative.h: In function 'apply_alternatives_module': >> arch/arm64/include/asm/alternative.h:31:1: warning: no return statement in function returning non-void [-Wreturn-type] 31 | static inline int apply_alternatives_module(void *start, size_t length) { } | ^~~~~~ vim +31 arch/arm64/include/asm/alternative.h 27 28 #ifdef CONFIG_MODULES 29 int apply_alternatives_module(void *start, size_t length); 30 #else > 31 static inline int apply_alternatives_module(void *start, size_t length) { } 32 #endif 33 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Hi Adrian, On Fri, 19 Sept 2025 at 14:23, Adrian Barnaś <abarnas@google.com> wrote: > > During module loading, check if there is a callback function used by the > alternatives specified in the '.altinstruction' ELF section and block > loading the module if such a function is present. > Why? AIUI, the issue being addressed is the fact that we cannot yet execute code from the module itself when alternatives are being applied, and so the callback must live in the core kernel, or in another module. So this is a really big hammer, given that it disallows all callback alternatives, including ones that we could easily support. I don't object to the approach per se, but please explain why this solution is reasonable (after you've explained the problem) > 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 | 4 ++-- > arch/arm64/kernel/alternative.c | 15 ++++++++++----- > arch/arm64/kernel/module.c | 8 ++++++-- > 3 files changed, 18 insertions(+), 9 deletions(-) > > diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h > index 00d97b8a757f..6eab2517c809 100644 > --- a/arch/arm64/include/asm/alternative.h > +++ b/arch/arm64/include/asm/alternative.h > @@ -26,9 +26,9 @@ 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) { } > #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..69fae94c843a 100644 > --- a/arch/arm64/kernel/alternative.c > +++ b/arch/arm64/kernel/alternative.c > @@ -139,15 +139,18 @@ 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; > alternative_cb_t alt_cb; > > for (alt = region->begin; alt < region->end; alt++) { > + if (ALT_HAS_CB(alt) && is_module) > + return -EPERM; Nit: is EPERM the appropriate error code here? > + > int nr_inst; > int cap = ALT_CAP(alt); > > @@ -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 5d6d228c6156..aeefc50229e3 100644 > --- a/arch/arm64/kernel/module.c > +++ b/arch/arm64/kernel/module.c > @@ -478,8 +478,12 @@ 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 >
On Fri, Sep 19, 2025 at 05:01:09PM +0200, Ard Biesheuvel wrote: >Hi Adrian, > >On Fri, 19 Sept 2025 at 14:23, Adrian Barnaś <abarnas@google.com> wrote: >> >> During module loading, check if there is a callback function used by the >> alternatives specified in the '.altinstruction' ELF section and block >> loading the module if such a function is present. >> > >Why? > >AIUI, the issue being addressed is the fact that we cannot yet execute >code from the module itself when alternatives are being applied, and >so the callback must live in the core kernel, or in another module. > >So this is a really big hammer, given that it disallows all callback >alternatives, including ones that we could easily support. Yes, it is true. What about we check if cb resides in kernel .text using core_kernel_text()? Pointer should be valid already since applying alternatives is done after relocation. We can shrink the hammer a bit. Then we allow the callback from inside the kernel to be called from the module but reject modules with callbacks that are invalid. Thanks, Adrian
© 2016 - 2025 Red Hat, Inc.