[PATCH] arm64/module: Support for patching modules during runtime

[PATCH] arm64/module: Support for patching modules during runtime

[fanqincui]

From: Fanqin Cui <cuifq1@chinatelecom.cn>

If use the ALTERNATIVE_CB interface in a kernel module to
patch code, the kernel will crash. The relevant log is as follows:

 Mem abort info:
   ESR = 0x000000008600000f
   EC = 0x21: IABT (current EL), IL = 32 bits
   SET = 0, FnV = 0
   EA = 0, S1PTW = 0
   FSC = 0x0f: level 3 permission fault
 swapper pgtable: 4k pages, 48-bit VAs, pgdp=00000013cfbed000
 [ffff80007b0b0000] pgd=0000000000000000, p4d=10000013d0d03003,
pud=1000000103175403, pmd=1000000115804403, pte=0068000116b77703
 Internal error: Oops: 000000008600000f [#1]  SMP

 Call trace:
  0xffff80007b0b0000 (P)
  apply_alternatives_module+0x48/0x7c
  module_finalize+0xc0/0x134
  load_module+0x15c0/0x1c08
  init_module_from_file+0x8c/0xcc
  __arm64_sys_finit_module+0x1c0/0x2d4
  invoke_syscall+0x48/0x110
  el0_svc_common.constprop.0+0xc0/0xe0
  do_el0_svc+0x1c/0x28
  el0_svc+0x34/0xf0
  el0t_64_sync_handler+0xa0/0xe4
  el0t_64_sync+0x198/0x19c
 Code: 00000000 00000000 00000000 00000000 (d503233f)
 ---[ end trace 0000000000000000 ]---

To avoid this problem, this commit supports add a new section.
When the module is loading, this section will be found and the
page table attributes will be set to executable state in advance.

Signed-off-by: Fanqin Cui <cuifq1@chinatelecom.cn>
---
 arch/arm64/kernel/module.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
index 40148d2725ce..2160b2877935 100644
--- a/arch/arm64/kernel/module.c
+++ b/arch/arm64/kernel/module.c
@@ -24,6 +24,7 @@
 #include <asm/scs.h>
 #include <asm/sections.h>
 #include <asm/text-patching.h>
+#include <asm-generic/set_memory.h>
 
 enum aarch64_reloc_op {
 	RELOC_OP_NONE,
@@ -477,6 +478,9 @@ int module_finalize(const Elf_Ehdr *hdr,
 	const Elf_Shdr *s;
 	int ret;
 
+	s = find_section(hdr, sechdrs, ".text.alternative_cb");
+	if (s && s->sh_size > PAGE_SIZE && PAGE_ALIGNED(s->sh_addr))
+		set_memory_x(s->sh_addr, s->sh_size >> PAGE_SHIFT);
 	s = find_section(hdr, sechdrs, ".altinstructions");
 	if (s)
 		apply_alternatives_module((void *)s->sh_addr, s->sh_size);
-- 
2.27.0

Re: [PATCH] arm64/module: Support for patching modules during runtime

[Will Deacon]

On Thu, Aug 07, 2025 at 03:27:00AM -0400, fanqincui@163.com wrote:
> From: Fanqin Cui <cuifq1@chinatelecom.cn>
> 
> If use the ALTERNATIVE_CB interface in a kernel module to
> patch code, the kernel will crash. The relevant log is as follows:
> 
>  Mem abort info:
>    ESR = 0x000000008600000f
>    EC = 0x21: IABT (current EL), IL = 32 bits
>    SET = 0, FnV = 0
>    EA = 0, S1PTW = 0
>    FSC = 0x0f: level 3 permission fault
>  swapper pgtable: 4k pages, 48-bit VAs, pgdp=00000013cfbed000
>  [ffff80007b0b0000] pgd=0000000000000000, p4d=10000013d0d03003,
> pud=1000000103175403, pmd=1000000115804403, pte=0068000116b77703
>  Internal error: Oops: 000000008600000f [#1]  SMP
> 
>  Call trace:
>   0xffff80007b0b0000 (P)
>   apply_alternatives_module+0x48/0x7c
>   module_finalize+0xc0/0x134
>   load_module+0x15c0/0x1c08
>   init_module_from_file+0x8c/0xcc
>   __arm64_sys_finit_module+0x1c0/0x2d4
>   invoke_syscall+0x48/0x110
>   el0_svc_common.constprop.0+0xc0/0xe0
>   do_el0_svc+0x1c/0x28
>   el0_svc+0x34/0xf0
>   el0t_64_sync_handler+0xa0/0xe4
>   el0t_64_sync+0x198/0x19c
>  Code: 00000000 00000000 00000000 00000000 (d503233f)
>  ---[ end trace 0000000000000000 ]---
> 
> To avoid this problem, this commit supports add a new section.
> When the module is loading, this section will be found and the
> page table attributes will be set to executable state in advance.
> 
> Signed-off-by: Fanqin Cui <cuifq1@chinatelecom.cn>
> ---
>  arch/arm64/kernel/module.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
> index 40148d2725ce..2160b2877935 100644
> --- a/arch/arm64/kernel/module.c
> +++ b/arch/arm64/kernel/module.c
> @@ -24,6 +24,7 @@
>  #include <asm/scs.h>
>  #include <asm/sections.h>
>  #include <asm/text-patching.h>
> +#include <asm-generic/set_memory.h>
>  
>  enum aarch64_reloc_op {
>  	RELOC_OP_NONE,
> @@ -477,6 +478,9 @@ int module_finalize(const Elf_Ehdr *hdr,
>  	const Elf_Shdr *s;
>  	int ret;
>  
> +	s = find_section(hdr, sechdrs, ".text.alternative_cb");
> +	if (s && s->sh_size > PAGE_SIZE && PAGE_ALIGNED(s->sh_addr))
> +		set_memory_x(s->sh_addr, s->sh_size >> PAGE_SHIFT);

Hmm, so the alternatives callback function lives in the module itself?
Which module does that? I'm a bit nervous about running module code
before the module has actually finished loading...

Does layout_sections() correctly map '.text.alternative_cb' as
executable later on?

Will

Re: [PATCH] arm64/module: Support for patching modules during runtime

[Marc Zyngier]

On Mon, 11 Aug 2025 08:37:32 +0100,
fanqincui <fanqincui@163.com> wrote:
> 
> Hi will,
> Yes, you are right. The alternative callback function lives inside the module.
> This callback function is actually similar to kvm_update_va_mask in KVM;
> 
> The module's callback function calculates some values based on
> the current CPU features and then performs the replacement.
> 
> The .text.alternative_cb section is actually marked as SHF_EXECINSTR | SHF_ALLOC
> during compilation, so intersections() includes this section and sets it as executable later.

I'm worried there is a chicken-and-egg problem here. What if the
callback itself requires patching via some other alternative? Is there
a guarantee that this always performed in the correct order?

	M.

-- 
Without deviation from the norm, progress is not possible.

Re:Re: [PATCH] arm64/module: Support for patching modules during runtime

[fanqincui]

Hi marc,
The callback function is designed by the developer. Developers need
to use the callback function to patch their own module code. Under
this premise, developers are responsible for providing the correct
callback function.
A correct callback function implementation does not require further
patching. Furthermore, the callback itself must be executable. If the
callback function has problems, the module's functionality will be affected.

Fanqin












At 2025-08-11 16:01:43, "Marc Zyngier" <maz@kernel.org> wrote:
>On Mon, 11 Aug 2025 08:37:32 +0100,
>fanqincui <fanqincui@163.com> wrote:
>> 
>> Hi will,
>> Yes, you are right. The alternative callback function lives inside the module.
>> This callback function is actually similar to kvm_update_va_mask in KVM;
>> 
>> The module's callback function calculates some values based on
>> the current CPU features and then performs the replacement.
>> 
>> The .text.alternative_cb section is actually marked as SHF_EXECINSTR | SHF_ALLOC
>> during compilation, so intersections() includes this section and sets it as executable later.
>
>I'm worried there is a chicken-and-egg problem here. What if the
>callback itself requires patching via some other alternative? Is there
>a guarantee that this always performed in the correct order?
>
>	M.
>
>-- 
>Without deviation from the norm, progress is not possible.

Re: [PATCH] arm64/module: Support for patching modules during runtime

[Marc Zyngier]

On Mon, 11 Aug 2025 09:32:19 +0100,
fanqincui <fanqincui@163.com> wrote:
> 
> 
> Hi marc,
> The callback function is designed by the developer. Developers need
> to use the callback function to patch their own module code. Under
> this premise, developers are responsible for providing the correct
> callback function.
> A correct callback function implementation does not require further
> patching.

Well, you can't know about that. We patch basic primitives such as
atomics, system register access, and plenty of other things. These
things need to interoperate with the rest of the kernel.

It's already difficult to guarantee inside the kernel itself. Having
it in random modules will be even harder.

> Furthermore, the callback itself must be executable. If the callback
> function has problems, the module's functionality will be affected.

Exactly. Hence my question.

	M.

-- 
Without deviation from the norm, progress is not possible.

Re:Re: [PATCH] arm64/module: Support for patching modules during runtime

[fanqincui]

>Well, you can't know about that. We patch basic primitives such as
>atomics, system register access, and plenty of other things. These
>things need to interoperate with the rest of the kernel.
>
>It's already difficult to guarantee inside the kernel itself. Having
>it in random modules will be even harder.
>


Okay, so the kernel patches you mentioned, are they already patched
when the module is installed? This doesn't conflict with the kernel patching.
I mean, the specific patching within the module is up to me. If the
chicken-and-egg problem you mentioned exist, module developers
should avoid it in their own code. 


I think the kernel should provide modules with the ability to patch
themselves, right?


Fanqin





At 2025-08-11 16:55:55, "Marc Zyngier" <maz@kernel.org> wrote:
>On Mon, 11 Aug 2025 09:32:19 +0100,
>fanqincui <fanqincui@163.com> wrote:
>> 
>> 
>> Hi marc,
>> The callback function is designed by the developer. Developers need
>> to use the callback function to patch their own module code. Under
>> this premise, developers are responsible for providing the correct
>> callback function.
>> A correct callback function implementation does not require further
>> patching.
>
>Well, you can't know about that. We patch basic primitives such as
>atomics, system register access, and plenty of other things. These
>things need to interoperate with the rest of the kernel.
>
>It's already difficult to guarantee inside the kernel itself. Having
>it in random modules will be even harder.
>
>> Furthermore, the callback itself must be executable. If the callback
>> function has problems, the module's functionality will be affected.
>
>Exactly. Hence my question.
>
>	M.
>
>-- 
>Without deviation from the norm, progress is not possible.

Re: [PATCH] arm64/module: Support for patching modules during runtime

[Marc Zyngier]

On Mon, 11 Aug 2025 10:57:44 +0100,
fanqincui <fanqincui@163.com> wrote:
>
> >Well, you can't know about that. We patch basic primitives such as
> >atomics, system register access, and plenty of other things. These
> >things need to interoperate with the rest of the kernel.
> >
> >It's already difficult to guarantee inside the kernel itself. Having
> >it in random modules will be even harder.
> >
> 
> 
> Okay, so the kernel patches you mentioned, are they already patched
> when the module is installed?

Yes.

> This doesn't conflict with the kernel patching.

In what sense?

> I mean, the specific patching within the module is up to me.

No.

> If the chicken-and-egg problem you mentioned exist, module
> developers should avoid it in their own code. 

We're not in the business of making the kernel more fragile and hard
to maintain than it already is. So either it *always* works, or it is
completely disallowed.

> I think the kernel should provide modules with the ability to patch
> themselves, right?

Only if it is safe to do so. Which is why I asked a question in my
initial reply, which you still haven't answered.

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH] arm64/module: Support for patching modules during runtime

[Will Deacon]

On Mon, Aug 11, 2025 at 09:01:43AM +0100, Marc Zyngier wrote:
> On Mon, 11 Aug 2025 08:37:32 +0100,
> fanqincui <fanqincui@163.com> wrote:
> > 
> > Hi will,
> > Yes, you are right. The alternative callback function lives inside the module.
> > This callback function is actually similar to kvm_update_va_mask in KVM;
> > 
> > The module's callback function calculates some values based on
> > the current CPU features and then performs the replacement.
> > 
> > The .text.alternative_cb section is actually marked as SHF_EXECINSTR | SHF_ALLOC
> > during compilation, so intersections() includes this section and sets it as executable later.
> 
> I'm worried there is a chicken-and-egg problem here. What if the
> callback itself requires patching via some other alternative? Is there
> a guarantee that this always performed in the correct order?

Maybe we should just reject loading modules that have alternative
callbacks that don't reside in the kernel text? I _think_ that should
cover all the in-tree users, although I didn't get a reply to my
question asking which module triggered this bug report.

Will

Re: [PATCH] arm64/module: Support for patching modules during runtime

[Mark Rutland]

On Mon, Aug 11, 2025 at 01:05:06PM +0100, Will Deacon wrote:
> On Mon, Aug 11, 2025 at 09:01:43AM +0100, Marc Zyngier wrote:
> > On Mon, 11 Aug 2025 08:37:32 +0100,
> > fanqincui <fanqincui@163.com> wrote:
> > > 
> > > Hi will,
> > > Yes, you are right. The alternative callback function lives inside the module.
> > > This callback function is actually similar to kvm_update_va_mask in KVM;
> > > 
> > > The module's callback function calculates some values based on
> > > the current CPU features and then performs the replacement.
> > > 
> > > The .text.alternative_cb section is actually marked as SHF_EXECINSTR | SHF_ALLOC
> > > during compilation, so intersections() includes this section and sets it as executable later.
> > 
> > I'm worried there is a chicken-and-egg problem here. What if the
> > callback itself requires patching via some other alternative? Is there
> > a guarantee that this always performed in the correct order?
> 
> Maybe we should just reject loading modules that have alternative
> callbacks that don't reside in the kernel text? 

I think that would be sensible. We never *intended* to support arbitrary
callbacks in modules, and if that's something people want, they need to
provide some actual justification.

> I _think_ that should cover all the in-tree users, although I didn't
> get a reply to my question asking which module triggered this bug
> report.

To the best of my knowledge, that covers all in-tree users. From a quick
grep for 'alternative_cb' and 'ALTERNATIVE_CB' in v6.17-rc1, all of the
patching functions are non-modular. AFAICT the only one we export is
alt_cb_patch_nops().

Mark.