ROX allocations broke livepatch for modules since 6.13-rc1

ROX allocations broke livepatch for modules since 6.13-rc1

[Marek Maślanka]

Hi Mike and others,

I discovered that the patch "[v7,4/8] module: prepare to handle ROX allocations
for text" has disrupted livepatch functionality. Specifically, this occurs when
livepatch is prepared to patch a kernel module and when the livepatch module
contains a "special" relocation section named
".klp.rela.<MODULE_NAME>.<SECTION_NAME>" to access local symbols.
The problem is caused by this changes:

@@ -2454,7 +2497,24 @@  static int post_relocation(struct module *mod,
const struct load_info *info)
  add_kallsyms(mod, info);

  /* Arch-specific module finalizing. */
- return module_finalize(info->hdr, info->sechdrs, mod);
+ ret = module_finalize(info->hdr, info->sechdrs, mod);
+ if (ret)
+ return ret;
+
+ for_each_mod_mem_type(type) {
+ struct module_memory *mem = &mod->mem[type];
+
+ if (mem->is_rox) {
+ if (!execmem_update_copy(mem->base, mem->rw_copy,
+ mem->size))
+ return -ENOMEM;
+
+ vfree(mem->rw_copy);
+ mem->rw_copy = NULL;
+ }
+ }
+
+ return module_post_finalize(info->hdr, info->sechdrs, mod);
 }

Specifically these lines:
+ vfree(mem->rw_copy);
+ mem->rw_copy = NULL;
which frees the "mem->rw_copy" too early. It's called from:
load_module (kernel/module/main.c:3312)
  post_relocation

The "mem->rw_copy" is needed later:
load_module (kernel/module/main.c:3339)
  prepare_coming_module
    klp_module_coming
      klp_init_object_loaded
        klp_apply_object_relocs
          klp_write_object_relocs
            klp_write_section_relocs
              apply_relocate_add
                write_relocate_add
                  __write_relocate_add
                    module_writable_address
                      __module_writable_address
                        return loc + (mem->rw_copy - mem->base);
                                                       ^

Here's example OOPS:
[   25.823395] deku: loading out-of-tree module taints kernel.
[   25.823521] deku: tainting kernel with TAINT_LIVEPATCH
[   25.827238] BUG: unable to handle page fault for address: 00000000000012ba
[   25.827819] #PF: supervisor read access in kernel mode
[   25.828153] #PF: error_code(0x0000) - not-present page
[   25.828588] PGD 0 P4D 0
[   25.829063] Oops: Oops: 0000 [#1] PREEMPT SMP NOPTI
[   25.829742] CPU: 2 UID: 0 PID: 452 Comm: insmod Tainted: G
 O  K    6.13.0-rc4-00078-g059dd502b263 #7820
[   25.830417] Tainted: [O]=OOT_MODULE, [K]=LIVEPATCH
[   25.830768] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS 1.16.0-20220807_005459-localhost 04/01/2014
[   25.831651] RIP: 0010:memcmp+0x24/0x60
[   25.832190] Code: 90 90 90 90 90 90 f3 0f 1e fa 49 89 f0 48 89 d6
48 83 fa 07 77 3a 31 d2 48 85 f6 74 1a 31 c0 eb 09 48 83 c0 01 48 39
c6 74 0d <0f> b6 14 07 41 0f b6 0c 00 29 ca 74 ea 89 d0 c3 cc cc cc cc
48 83
[   25.833378] RSP: 0018:ffffa40b403a3ae8 EFLAGS: 00000246
[   25.833637] RAX: 0000000000000000 RBX: ffff93bc81d8e700 RCX: ffffffffc0202000
[   25.834072] RDX: 0000000000000000 RSI: 0000000000000004 RDI: 00000000000012ba
[   25.834548] RBP: ffffa40b403a3b68 R08: ffffa40b403a3b30 R09: 0000004a00000002
[   25.835088] R10: ffffffffffffd222 R11: f000000000000000 R12: 0000000000000000
[   25.835666] R13: ffffffffc02032ba R14: ffffffffc007d1e0 R15: 0000000000000004
[   25.836139] FS:  00007fecef8c3080(0000) GS:ffff93bc8f900000(0000)
knlGS:0000000000000000
[   25.836519] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   25.836977] CR2: 00000000000012ba CR3: 0000000002f24000 CR4: 00000000000006f0
[   25.837442] Call Trace:
[   25.838297]  <TASK>
[   25.838803]  ? __die+0x29/0x70
[   25.839067]  ? page_fault_oops+0x15f/0x450
[   25.839299]  ? exc_page_fault+0x6e/0x160
[   25.839506]  ? asm_exc_page_fault+0x26/0x30
[   25.839978]  ? uinput_ioctl_handler_isra_0+0x120/0xe66 [deku]
[   25.840519]  ? uinput_misc_exit+0x3e0/0x3e0 [uinput]
[   25.840892]  ? memcmp+0x24/0x60
[   25.841083]  __write_relocate_add.constprop.0+0xc7/0x2b0
[   25.841460]  ? __pfx_text_poke+0x10/0x10
[   25.841701]  apply_relocate_add+0x75/0xa0
[   25.841973]  klp_write_section_relocs+0x10e/0x140
[   25.842304]  klp_write_object_relocs+0x70/0xa0
[   25.842682]  klp_init_object_loaded+0x21/0xf0
[   25.842972]  klp_enable_patch+0x43d/0x900
[   25.843227]  ? deku_exit+0x30/0x30 [deku]
[   25.843572]  do_one_initcall+0x4c/0x220
[   25.843936]  ? do_init_module+0x28/0x260
[   25.844186]  do_init_module+0x6a/0x260
[   25.844423]  init_module_from_file+0x9c/0xe0
[   25.844702]  idempotent_init_module+0x172/0x270
[   25.845008]  __x64_sys_finit_module+0x69/0xc0
[   25.845253]  do_syscall_64+0x9e/0x1a0
[   25.845498]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
[   25.846056] RIP: 0033:0x7fecef9eb25d
[   25.846444] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e
fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24
08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 8b bb 0d 00 f7 d8 64 89
01 48
[   25.847563] RSP: 002b:00007ffd0c5d6de8 EFLAGS: 00000246 ORIG_RAX:
0000000000000139
[   25.848082] RAX: ffffffffffffffda RBX: 000055b03f05e470 RCX: 00007fecef9eb25d
[   25.848456] RDX: 0000000000000000 RSI: 000055b001e74e52 RDI: 0000000000000003
[   25.848969] RBP: 00007ffd0c5d6ea0 R08: 0000000000000040 R09: 0000000000004100
[   25.849411] R10: 00007fecefac7b20 R11: 0000000000000246 R12: 000055b001e74e52
[   25.849905] R13: 0000000000000000 R14: 000055b03f05e440 R15: 0000000000000000
[   25.850336]  </TASK>
[   25.850553] Modules linked in: deku(OK+) uinput
[   25.851408] CR2: 00000000000012ba
[   25.852085] ---[ end trace 0000000000000000 ]---

Please let me know if you need help to test or reproduce this issue.

Best regards,
Marek

Re: ROX allocations broke livepatch for modules since 6.13-rc1

[Petr Pavlu]

On 12/30/24 01:09, Marek Maślanka wrote:
> Hi Mike and others,
> 
> I discovered that the patch "[v7,4/8] module: prepare to handle ROX allocations
> for text" has disrupted livepatch functionality. Specifically, this occurs when
> livepatch is prepared to patch a kernel module and when the livepatch module
> contains a "special" relocation section named
> ".klp.rela.<MODULE_NAME>.<SECTION_NAME>" to access local symbols.

Thank you for the report. It is possible for me to reproduce the issue
on my system. An annoying part is to create the
.klp.rela.<objname>.<secname> data, for which I eventually used one
floating variant of klp-convert [1]. To hit the problem, <objname> must
point to an object that is different from vmlinux. Such relocations are
processed by the livepatch code later than regular module relocations,
as you pointed out after mod->rw_copy is already reset.

I think the bug should be addressed in principle by Mike's recently
posted rework of the feature [2] but unfortunately, its current version
makes my system also unbootable [3].

My reproducer:

$ cat test_klp_livepatch.c
#include <linux/kernel.h>
#include <linux/livepatch.h>
#include <linux/module.h>
#include <linux/seq_file.h>
extern __attribute__((weak)) void *xfs_btree_new_root asm("\".klp.sym.rela.xfs.xfs.xfs_btree_new_root.0\"");
static int livepatch_cmdline_proc_show(struct seq_file *m, void *v) {
        seq_printf(m, "live patched: xfs_btree_new_root=%p\n", &xfs_btree_new_root); return 0; }
static struct klp_func funcs[] = { { .old_name = "cmdline_proc_show", .new_func = livepatch_cmdline_proc_show }, { } };
static struct klp_func xfs_funcs[] = { { } };
static struct klp_object objs[] = { { .funcs = funcs, }, { .name = "xfs", .funcs = xfs_funcs, }, { } };
static struct klp_patch patch = { .mod = THIS_MODULE, .objs = objs, };
static int test_klp_livepatch_init(void) { return klp_enable_patch(&patch); }
static void test_klp_livepatch_exit(void) { }
module_init(test_klp_livepatch_init);
module_exit(test_klp_livepatch_exit);
MODULE_LICENSE("GPL");
MODULE_INFO(livepatch, "Y");
MODULE_AUTHOR("Petr Pavlu <petr.pavlu@suse.com>");
MODULE_DESCRIPTION("Livepatch test: livepatch module");

$ make ...
$ klp-convert test_klp_livepatch.ko test_klp_livepatch.conv.ko  # [1]
# modprobe -a xfs ./test_klp_livepatch.conv.ko

[1] https://lore.kernel.org/live-patching/20240827123052.9002-3-lhruska@suse.cz/
[2] https://lore.kernel.org/linux-modules/20241227072825.1288491-1-rppt@kernel.org/
[3] https://lore.kernel.org/linux-modules/86eba318-464b-4b9b-a79e-64039b17be34@lucifer.local/

-- 
Thanks,
Petr

Re: ROX allocations broke livepatch for modules since 6.13-rc1

[Petr Pavlu]

On 1/5/25 15:51, Petr Pavlu wrote:
> On 12/30/24 01:09, Marek Maślanka wrote:
>> Hi Mike and others,
>>
>> I discovered that the patch "[v7,4/8] module: prepare to handle ROX allocations
>> for text" has disrupted livepatch functionality. Specifically, this occurs when
>> livepatch is prepared to patch a kernel module and when the livepatch module
>> contains a "special" relocation section named
>> ".klp.rela.<MODULE_NAME>.<SECTION_NAME>" to access local symbols.
> 
> Thank you for the report. It is possible for me to reproduce the issue
> on my system. An annoying part is to create the
> .klp.rela.<objname>.<secname> data, for which I eventually used one
> floating variant of klp-convert [1]. To hit the problem, <objname> must
> point to an object that is different from vmlinux. Such relocations are
> processed by the livepatch code later than regular module relocations,
> as you pointed out after mod->rw_copy is already reset.
> 
> I think the bug should be addressed in principle by Mike's recently
> posted rework of the feature [2] but unfortunately, its current version
> makes my system also unbootable [3].

A simpler fix could be:

diff --git a/include/linux/module.h b/include/linux/module.h
index 94acbacdcdf1..b3a643435357 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -773,7 +773,8 @@ void *__module_writable_address(struct module *mod, void *loc);
 
 static inline void *module_writable_address(struct module *mod, void *loc)
 {
-	if (!IS_ENABLED(CONFIG_ARCH_HAS_EXECMEM_ROX) || !mod)
+	if (!IS_ENABLED(CONFIG_ARCH_HAS_EXECMEM_ROX) || !mod ||
+	    mod->state != MODULE_STATE_UNFORMED)
 		return loc;
 	return __module_writable_address(mod, loc);
 }

Hm, is it expected that Mike's rework to drop rw_copy will make it into
6.13 or should I properly post this minimal fix for review?

-- 
Thanks,
Petr

[PATCH] module: Fix writing of livepatch relocations in ROX text

[Petr Pavlu]

A livepatch module can contain a special relocation section
.klp.rela.<objname>.<secname> to apply its relocations at the appropriate
time and to additionally access local and unexported symbols. When
<objname> points to another module, such relocations are processed
separately from the regular module relocation process. For instance, only
when the target <objname> actually becomes loaded.

With CONFIG_STRICT_MODULE_RWX, when the livepatch core decides to apply
these relocations, their processing results in the following bug:

[   25.827238] BUG: unable to handle page fault for address: 00000000000012ba
[   25.827819] #PF: supervisor read access in kernel mode
[   25.828153] #PF: error_code(0x0000) - not-present page
[   25.828588] PGD 0 P4D 0
[   25.829063] Oops: Oops: 0000 [#1] PREEMPT SMP NOPTI
[   25.829742] CPU: 2 UID: 0 PID: 452 Comm: insmod Tainted: G O  K    6.13.0-rc4-00078-g059dd502b263 #7820
[   25.830417] Tainted: [O]=OOT_MODULE, [K]=LIVEPATCH
[   25.830768] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-20220807_005459-localhost 04/01/2014
[   25.831651] RIP: 0010:memcmp+0x24/0x60
[   25.832190] Code: [...]
[   25.833378] RSP: 0018:ffffa40b403a3ae8 EFLAGS: 00000246
[   25.833637] RAX: 0000000000000000 RBX: ffff93bc81d8e700 RCX: ffffffffc0202000
[   25.834072] RDX: 0000000000000000 RSI: 0000000000000004 RDI: 00000000000012ba
[   25.834548] RBP: ffffa40b403a3b68 R08: ffffa40b403a3b30 R09: 0000004a00000002
[   25.835088] R10: ffffffffffffd222 R11: f000000000000000 R12: 0000000000000000
[   25.835666] R13: ffffffffc02032ba R14: ffffffffc007d1e0 R15: 0000000000000004
[   25.836139] FS:  00007fecef8c3080(0000) GS:ffff93bc8f900000(0000) knlGS:0000000000000000
[   25.836519] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   25.836977] CR2: 00000000000012ba CR3: 0000000002f24000 CR4: 00000000000006f0
[   25.837442] Call Trace:
[   25.838297]  <TASK>
[   25.841083]  __write_relocate_add.constprop.0+0xc7/0x2b0
[   25.841701]  apply_relocate_add+0x75/0xa0
[   25.841973]  klp_write_section_relocs+0x10e/0x140
[   25.842304]  klp_write_object_relocs+0x70/0xa0
[   25.842682]  klp_init_object_loaded+0x21/0xf0
[   25.842972]  klp_enable_patch+0x43d/0x900
[   25.843572]  do_one_initcall+0x4c/0x220
[   25.844186]  do_init_module+0x6a/0x260
[   25.844423]  init_module_from_file+0x9c/0xe0
[   25.844702]  idempotent_init_module+0x172/0x270
[   25.845008]  __x64_sys_finit_module+0x69/0xc0
[   25.845253]  do_syscall_64+0x9e/0x1a0
[   25.845498]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
[   25.846056] RIP: 0033:0x7fecef9eb25d
[   25.846444] Code: [...]
[   25.847563] RSP: 002b:00007ffd0c5d6de8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
[   25.848082] RAX: ffffffffffffffda RBX: 000055b03f05e470 RCX: 00007fecef9eb25d
[   25.848456] RDX: 0000000000000000 RSI: 000055b001e74e52 RDI: 0000000000000003
[   25.848969] RBP: 00007ffd0c5d6ea0 R08: 0000000000000040 R09: 0000000000004100
[   25.849411] R10: 00007fecefac7b20 R11: 0000000000000246 R12: 000055b001e74e52
[   25.849905] R13: 0000000000000000 R14: 000055b03f05e440 R15: 0000000000000000
[   25.850336]  </TASK>
[   25.850553] Modules linked in: deku(OK+) uinput
[   25.851408] CR2: 00000000000012ba
[   25.852085] ---[ end trace 0000000000000000 ]---

The problem is that the .klp.rela.<objname>.<secname> relocations are
processed after the module was already formed and mod->rw_copy was reset.
However, the code in __write_relocate_add() calls module_writable_address()
which translates the target address 'loc' still to
'loc + (mem->rw_copy - mem->base)', with mem->rw_copy now being 0.

Fix the problem by returning directly 'loc' in module_writable_address()
when the module is already formed. Function __write_relocate_add() knows to
use text_poke() in such a case.

Fixes: 0c133b1e78cd ("module: prepare to handle ROX allocations for text")
Reported-by: Marek Maslanka <mmaslanka@google.com>
Closes: https://lore.kernel.org/linux-modules/CAGcaFA2hdThQV6mjD_1_U+GNHThv84+MQvMWLgEuX+LVbAyDxg@mail.gmail.com/
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
 include/linux/module.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 94acbacdcdf1..b3a643435357 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -773,7 +773,8 @@ void *__module_writable_address(struct module *mod, void *loc);
 
 static inline void *module_writable_address(struct module *mod, void *loc)
 {
-	if (!IS_ENABLED(CONFIG_ARCH_HAS_EXECMEM_ROX) || !mod)
+	if (!IS_ENABLED(CONFIG_ARCH_HAS_EXECMEM_ROX) || !mod ||
+	    mod->state != MODULE_STATE_UNFORMED)
 		return loc;
 	return __module_writable_address(mod, loc);
 }

base-commit: 9d89551994a430b50c4fffcb1e617a057fa76e20
-- 
2.43.0

Re: [PATCH] module: Fix writing of livepatch relocations in ROX text

[Petr Mladek]

On Tue 2025-01-07 16:34:57, Petr Pavlu wrote:
> A livepatch module can contain a special relocation section
> .klp.rela.<objname>.<secname> to apply its relocations at the appropriate
> time and to additionally access local and unexported symbols. When
> <objname> points to another module, such relocations are processed
> separately from the regular module relocation process. For instance, only
> when the target <objname> actually becomes loaded.
> 
> With CONFIG_STRICT_MODULE_RWX, when the livepatch core decides to apply
> these relocations, their processing results in the following bug:
> 
> [   25.827238] BUG: unable to handle page fault for address: 00000000000012ba
> [   25.827819] #PF: supervisor read access in kernel mode
> [   25.828153] #PF: error_code(0x0000) - not-present page
> [   25.828588] PGD 0 P4D 0
> [   25.829063] Oops: Oops: 0000 [#1] PREEMPT SMP NOPTI
> [   25.829742] CPU: 2 UID: 0 PID: 452 Comm: insmod Tainted: G O  K    6.13.0-rc4-00078-g059dd502b263 #7820
> [   25.830417] Tainted: [O]=OOT_MODULE, [K]=LIVEPATCH
> [   25.830768] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.0-20220807_005459-localhost 04/01/2014
> [   25.831651] RIP: 0010:memcmp+0x24/0x60
> [   25.832190] Code: [...]
> [   25.833378] RSP: 0018:ffffa40b403a3ae8 EFLAGS: 00000246
> [   25.833637] RAX: 0000000000000000 RBX: ffff93bc81d8e700 RCX: ffffffffc0202000
> [   25.834072] RDX: 0000000000000000 RSI: 0000000000000004 RDI: 00000000000012ba
> [   25.834548] RBP: ffffa40b403a3b68 R08: ffffa40b403a3b30 R09: 0000004a00000002
> [   25.835088] R10: ffffffffffffd222 R11: f000000000000000 R12: 0000000000000000
> [   25.835666] R13: ffffffffc02032ba R14: ffffffffc007d1e0 R15: 0000000000000004
> [   25.836139] FS:  00007fecef8c3080(0000) GS:ffff93bc8f900000(0000) knlGS:0000000000000000
> [   25.836519] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   25.836977] CR2: 00000000000012ba CR3: 0000000002f24000 CR4: 00000000000006f0
> [   25.837442] Call Trace:
> [   25.838297]  <TASK>
> [   25.841083]  __write_relocate_add.constprop.0+0xc7/0x2b0
> [   25.841701]  apply_relocate_add+0x75/0xa0
> [   25.841973]  klp_write_section_relocs+0x10e/0x140
> [   25.842304]  klp_write_object_relocs+0x70/0xa0
> [   25.842682]  klp_init_object_loaded+0x21/0xf0
> [   25.842972]  klp_enable_patch+0x43d/0x900
> [   25.843572]  do_one_initcall+0x4c/0x220
> [   25.844186]  do_init_module+0x6a/0x260
> [   25.844423]  init_module_from_file+0x9c/0xe0
> [   25.844702]  idempotent_init_module+0x172/0x270
> [   25.845008]  __x64_sys_finit_module+0x69/0xc0
> [   25.845253]  do_syscall_64+0x9e/0x1a0
> [   25.845498]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> [   25.846056] RIP: 0033:0x7fecef9eb25d
> [   25.846444] Code: [...]
> [   25.847563] RSP: 002b:00007ffd0c5d6de8 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
> [   25.848082] RAX: ffffffffffffffda RBX: 000055b03f05e470 RCX: 00007fecef9eb25d
> [   25.848456] RDX: 0000000000000000 RSI: 000055b001e74e52 RDI: 0000000000000003
> [   25.848969] RBP: 00007ffd0c5d6ea0 R08: 0000000000000040 R09: 0000000000004100
> [   25.849411] R10: 00007fecefac7b20 R11: 0000000000000246 R12: 000055b001e74e52
> [   25.849905] R13: 0000000000000000 R14: 000055b03f05e440 R15: 0000000000000000
> [   25.850336]  </TASK>
> [   25.850553] Modules linked in: deku(OK+) uinput
> [   25.851408] CR2: 00000000000012ba
> [   25.852085] ---[ end trace 0000000000000000 ]---
> 
> The problem is that the .klp.rela.<objname>.<secname> relocations are
> processed after the module was already formed and mod->rw_copy was reset.
> However, the code in __write_relocate_add() calls module_writable_address()
> which translates the target address 'loc' still to
> 'loc + (mem->rw_copy - mem->base)', with mem->rw_copy now being 0.
> 
> Fix the problem by returning directly 'loc' in module_writable_address()
> when the module is already formed. Function __write_relocate_add() knows to
> use text_poke() in such a case.
> 
> Fixes: 0c133b1e78cd ("module: prepare to handle ROX allocations for text")
> Reported-by: Marek Maslanka <mmaslanka@google.com>
> Closes: https://lore.kernel.org/linux-modules/CAGcaFA2hdThQV6mjD_1_U+GNHThv84+MQvMWLgEuX+LVbAyDxg@mail.gmail.com/
> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>

The fix makes sense. I could confirm that it fixes the problem
and the livepatch relocations works again.

I have tested it on x86_64 with current Linus' origin/master
and the selftest from the patchset adding klp-convert tool, see
https://lore.kernel.org/r/20240827123052.9002-7-lhruska@suse.cz

Reviewed-by: Petr Mladek <pmladek@suse.com>
Tested-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr