module_enable_x() has nothing to do with CONFIG_ARCH_HAS_STRICT_MODULE_RWX
allthough by coincidence architectures who need module_enable_x() are
selection CONFIG_ARCH_HAS_STRICT_MODULE_RWX.
Enable module_enable_x() for everyone everytime. If an architecture
already has module text set executable, it's a no-op.
Only check end boundary if CONFIG_STRICT_MODULE_RWX is set, and
make sure we entirely get the last page when the boundary is not
aligned. When CONFIG_STRICT_MODULE_RWX is not selected, it is not
a big deal to have the start of data as executable.
Signed-off-by: Christophe Leroy <[email protected]>
---
kernel/module.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 24dab046e16c..44ed39cbbd17 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -70,9 +70,9 @@
/*
* Modules' sections will be aligned on page boundaries
* to ensure complete separation of code and data, but
- * only when CONFIG_ARCH_HAS_STRICT_MODULE_RWX=y
+ * only when CONFIG_STRICT_MODULE_RWX=y
*/
-#ifdef CONFIG_ARCH_HAS_STRICT_MODULE_RWX
+#ifdef CONFIG_STRICT_MODULE_RWX
# define debug_align(X) ALIGN(X, PAGE_SIZE)
#else
# define debug_align(X) (X)
@@ -1955,14 +1955,15 @@ static void mod_sysfs_teardown(struct module *mod)
* CONFIG_STRICT_MODULE_RWX block below because they are needed regardless of
* whether we are strict.
*/
-#ifdef CONFIG_ARCH_HAS_STRICT_MODULE_RWX
static void frob_text(const struct module_layout *layout,
int (*set_memory)(unsigned long start, int num_pages))
{
BUG_ON((unsigned long)layout->base & (PAGE_SIZE-1));
+#ifdef CONFIG_STRICT_MODULE_RWX
BUG_ON((unsigned long)layout->text_size & (PAGE_SIZE-1));
+#endif
set_memory((unsigned long)layout->base,
- layout->text_size >> PAGE_SHIFT);
+ ((layout->text_size - 1) >> PAGE_SHIFT) + 1);
}
static void module_enable_x(const struct module *mod)
@@ -1970,9 +1971,6 @@ static void module_enable_x(const struct module *mod)
frob_text(&mod->core_layout, set_memory_x);
frob_text(&mod->init_layout, set_memory_x);
}
-#else /* !CONFIG_ARCH_HAS_STRICT_MODULE_RWX */
-static void module_enable_x(const struct module *mod) { }
-#endif /* CONFIG_ARCH_HAS_STRICT_MODULE_RWX */
#ifdef CONFIG_STRICT_MODULE_RWX
static void frob_rodata(const struct module_layout *layout,
--
2.33.1
debug_align() was added by commit 84e1c6bb38eb ("x86: Add RO/NX
protection for loadable kernel modules")
At that time the config item was CONFIG_DEBUG_SET_MODULE_RONX.
But nowadays it has changed to CONFIG_STRICT_MODULE_RWX and
debug_align() is confusing because it has nothing to do with
DEBUG.
Rename it section_align()
While at it, use PAGE_ALIGN(x) instead of ALIGN(x, PAGE_SIZE).
Signed-off-by: Christophe Leroy <[email protected]>
---
kernel/module.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 44ed39cbbd17..fb30249a05bb 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -73,9 +73,9 @@
* only when CONFIG_STRICT_MODULE_RWX=y
*/
#ifdef CONFIG_STRICT_MODULE_RWX
-# define debug_align(X) ALIGN(X, PAGE_SIZE)
+# define section_align(X) PAGE_ALIGN(X)
#else
-# define debug_align(X) (X)
+# define section_align(X) (X)
#endif
/* If this is set, the section belongs in the init part of the module */
@@ -2454,19 +2454,19 @@ static void layout_sections(struct module *mod, struct load_info *info)
}
switch (m) {
case 0: /* executable */
- mod->core_layout.size = debug_align(mod->core_layout.size);
+ mod->core_layout.size = section_align(mod->core_layout.size);
mod->core_layout.text_size = mod->core_layout.size;
break;
case 1: /* RO: text and ro-data */
- mod->core_layout.size = debug_align(mod->core_layout.size);
+ mod->core_layout.size = section_align(mod->core_layout.size);
mod->core_layout.ro_size = mod->core_layout.size;
break;
case 2: /* RO after init */
- mod->core_layout.size = debug_align(mod->core_layout.size);
+ mod->core_layout.size = section_align(mod->core_layout.size);
mod->core_layout.ro_after_init_size = mod->core_layout.size;
break;
case 4: /* whole core */
- mod->core_layout.size = debug_align(mod->core_layout.size);
+ mod->core_layout.size = section_align(mod->core_layout.size);
break;
}
}
@@ -2488,11 +2488,11 @@ static void layout_sections(struct module *mod, struct load_info *info)
}
switch (m) {
case 0: /* executable */
- mod->init_layout.size = debug_align(mod->init_layout.size);
+ mod->init_layout.size = section_align(mod->init_layout.size);
mod->init_layout.text_size = mod->init_layout.size;
break;
case 1: /* RO: text and ro-data */
- mod->init_layout.size = debug_align(mod->init_layout.size);
+ mod->init_layout.size = section_align(mod->init_layout.size);
mod->init_layout.ro_size = mod->init_layout.size;
break;
case 2:
@@ -2503,7 +2503,7 @@ static void layout_sections(struct module *mod, struct load_info *info)
mod->init_layout.ro_after_init_size = mod->init_layout.ro_size;
break;
case 4: /* whole init */
- mod->init_layout.size = debug_align(mod->init_layout.size);
+ mod->init_layout.size = section_align(mod->init_layout.size);
break;
}
}
@@ -2724,7 +2724,7 @@ static void layout_symtab(struct module *mod, struct load_info *info)
mod->core_layout.size += strtab_size;
info->core_typeoffs = mod->core_layout.size;
mod->core_layout.size += ndst * sizeof(char);
- mod->core_layout.size = debug_align(mod->core_layout.size);
+ mod->core_layout.size = section_align(mod->core_layout.size);
/* Put string table section at end of init part of module. */
strsect->sh_flags |= SHF_ALLOC;
@@ -2739,7 +2739,7 @@ static void layout_symtab(struct module *mod, struct load_info *info)
mod->init_layout.size += sizeof(struct mod_kallsyms);
info->init_typeoffs = mod->init_layout.size;
mod->init_layout.size += nsrc * sizeof(char);
- mod->init_layout.size = debug_align(mod->init_layout.size);
+ mod->init_layout.size = section_align(mod->init_layout.size);
}
/*
--
2.33.1
On Thu, Feb 03, 2022 at 08:23:25PM +0000, Christophe Leroy wrote:
> module_enable_x() has nothing to do with CONFIG_ARCH_HAS_STRICT_MODULE_RWX
> allthough by coincidence architectures who need module_enable_x() are
> selection CONFIG_ARCH_HAS_STRICT_MODULE_RWX.
>
> Enable module_enable_x() for everyone everytime. If an architecture
> already has module text set executable, it's a no-op.
>
> Only check end boundary if CONFIG_STRICT_MODULE_RWX is set, and
> make sure we entirely get the last page when the boundary is not
> aligned. When CONFIG_STRICT_MODULE_RWX is not selected, it is not
> a big deal to have the start of data as executable.
>
> Signed-off-by: Christophe Leroy <[email protected]>
Thanks!
Both patches look good to me! I can merge this once Aaron's
then your's and then Michal's KEXEC stuff gets merged.
Luis
Greeting,
FYI, we noticed the following commit (built with clang-15):
commit: 64aee03f98360d482e574a3c3ab487af05e84830 ("[RFC PATCH 1/2] modules: Make module_enable_x() independant of CONFIG_ARCH_HAS_STRICT_MODULE_RWX")
url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/modules-Make-module_enable_x-independant-of-CONFIG_ARCH_HAS_STRICT_MODULE_RWX/20220204-042509
base: https://git.kernel.org/cgit/linux/kernel/git/mcgrof/linux.git modules-next
patch link: https://lore.kernel.org/linux-modules/203348805c9ac9851d8939d15cb9802ef047b5e2.1643919758.git.christophe.leroy@csgroup.eu
in testcase: boot
on test machine: qemu-system-i386 -enable-kvm -cpu SandyBridge -smp 2 -m 4G
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
+-----------------------------------------------------------------+------------+------------+
| | a97ac8cb24 | 64aee03f98 |
+-----------------------------------------------------------------+------------+------------+
| WARNING:at_arch/x86/mm/pat/set_memory.c:#__cpa_process_fault | 0 | 43 |
| EIP:__cpa_process_fault | 0 | 43 |
+-----------------------------------------------------------------+------------+------------+
If you fix the issue, kindly add following tag
Reported-by: kernel test robot <[email protected]>
[ 35.430975][ T206] WARNING: CPU: 0 PID: 206 at arch/x86/mm/pat/set_memory.c:1514 __cpa_process_fault (set_memory.c:?)
[ 35.432157][ T206] Modules linked in: serio_raw parport_pc parport qemu_fw_cfg
[ 35.433074][ T206] CPU: 0 PID: 206 Comm: systemd-udevd Tainted: G W 5.16.0-06526-g64aee03f9836 #1
[ 35.434242][ T206] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
[ 35.435308][ T206] EIP: __cpa_process_fault (set_memory.c:?)
[ 35.435965][ T206] Code: 57 31 f6 46 b8 d0 97 5f c2 89 d7 89 f2 31 c9 56 e8 cd fb 0d 00 83 c4 04 8b 03 ff 30 57 68 59 55 02 c2 e8 3b cc 00 00 83 c4 0c <0f> 0b b8 e8 97 5f c2 89 f2 31 c9 56 e8 a7 fb 0d 00 83 c4 04 b8 f2
All code
========
0: 57 push %rdi
1: 31 f6 xor %esi,%esi
3: 46 b8 d0 97 5f c2 rex.RX mov $0xc25f97d0,%eax
9: 89 d7 mov %edx,%edi
b: 89 f2 mov %esi,%edx
d: 31 c9 xor %ecx,%ecx
f: 56 push %rsi
10: e8 cd fb 0d 00 callq 0xdfbe2
15: 83 c4 04 add $0x4,%esp
18: 8b 03 mov (%rbx),%eax
1a: ff 30 pushq (%rax)
1c: 57 push %rdi
1d: 68 59 55 02 c2 pushq $0xffffffffc2025559
22: e8 3b cc 00 00 callq 0xcc62
27: 83 c4 0c add $0xc,%esp
2a:* 0f 0b ud2 <-- trapping instruction
2c: b8 e8 97 5f c2 mov $0xc25f97e8,%eax
31: 89 f2 mov %esi,%edx
33: 31 c9 xor %ecx,%ecx
35: 56 push %rsi
36: e8 a7 fb 0d 00 callq 0xdfbe2
3b: 83 c4 04 add $0x4,%esp
3e: b8 .byte 0xb8
3f: f2 repnz
Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: b8 e8 97 5f c2 mov $0xc25f97e8,%eax
7: 89 f2 mov %esi,%edx
9: 31 c9 xor %ecx,%ecx
b: 56 push %rsi
c: e8 a7 fb 0d 00 callq 0xdfbb8
11: 83 c4 04 add $0x4,%esp
14: b8 .byte 0xb8
15: f2 repnz
[ 35.438110][ T206] EAX: 00000040 EBX: f56bfd34 ECX: 00000001 EDX: ffffffff
[ 35.438892][ T206] ESI: 00000001 EDI: f7493000 EBP: f56bfc80 ESP: f56bfc50
[ 35.439717][ T206] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068 EFLAGS: 00010296
[ 35.440580][ T206] CR0: 80050033 CR2: 02c5a19c CR3: 35202000 CR4: 000406d0
[ 35.441410][ T206] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[ 35.442237][ T206] DR6: fffe0ff0 DR7: 00000400
[ 35.442806][ T206] Call Trace:
[ 35.443196][ T206] ? do_raw_spin_lock (fbdev.c:?)
[ 35.443773][ T206] __change_page_attr_set_clr (set_memory.c:?)
[ 35.444439][ T206] ? find_held_lock (lockdep.c:?)
[ 35.444975][ T206] ? _vm_unmap_aliases+0x100/0x140
[ 35.445581][ T206] ? lock_release (fbdev.c:?)
[ 35.446160][ T206] ? _vm_unmap_aliases+0x100/0x140
[ 35.446782][ T206] ? mutex_unlock (fbdev.c:?)
[ 35.447323][ T206] change_page_attr_set_clr+0x15f/0x1c0
[ 35.447992][ T206] ? set_memory_nx (fbdev.c:?)
[ 35.448552][ T206] set_memory_ro (fbdev.c:?)
[ 35.449154][ T206] frob_text (module.c:?)
[ 35.449646][ T206] module_enable_ro (module.c:?)
[ 35.450238][ T206] complete_formation (module.c:?)
[ 35.450832][ T206] load_module (module.c:?)
[ 35.451430][ T206] __ia32_sys_finit_module (fbdev.c:?)
[ 35.452082][ T206] do_int80_syscall_32 (fbdev.c:?)
[ 35.452735][ T206] ? do_int80_syscall_32 (fbdev.c:?)
[ 35.453368][ T206] ? irqentry_exit (fbdev.c:?)
[ 35.453949][ T206] ? exc_page_fault (fbdev.c:?)
[ 35.454555][ T206] entry_INT80_32 (??:?)
[ 35.455147][ T206] EIP: 0xb7f0fa02
[ 35.455576][ T206] Code: 95 01 00 05 25 36 02 00 83 ec 14 8d 80 e8 99 ff ff 50 6a 02 e8 1f ff 00 00 c7 04 24 7f 00 00 00 e8 7e 87 01 00 66 90 90 cd 80 <c3> 8d b6 00 00 00 00 8d bc 27 00 00 00 00 8b 1c 24 c3 8d b6 00 00
All code
========
0: 95 xchg %eax,%ebp
1: 01 00 add %eax,(%rax)
3: 05 25 36 02 00 add $0x23625,%eax
8: 83 ec 14 sub $0x14,%esp
b: 8d 80 e8 99 ff ff lea -0x6618(%rax),%eax
11: 50 push %rax
12: 6a 02 pushq $0x2
14: e8 1f ff 00 00 callq 0xff38
19: c7 04 24 7f 00 00 00 movl $0x7f,(%rsp)
20: e8 7e 87 01 00 callq 0x187a3
25: 66 90 xchg %ax,%ax
27: 90 nop
28: cd 80 int $0x80
2a:* c3 retq <-- trapping instruction
2b: 8d b6 00 00 00 00 lea 0x0(%rsi),%esi
31: 8d bc 27 00 00 00 00 lea 0x0(%rdi,%riz,1),%edi
38: 8b 1c 24 mov (%rsp),%ebx
3b: c3 retq
3c: 8d .byte 0x8d
3d: b6 00 mov $0x0,%dh
...
Code starting with the faulting instruction
===========================================
0: c3 retq
1: 8d b6 00 00 00 00 lea 0x0(%rsi),%esi
7: 8d bc 27 00 00 00 00 lea 0x0(%rdi,%riz,1),%edi
e: 8b 1c 24 mov (%rsp),%ebx
11: c3 retq
12: 8d .byte 0x8d
13: b6 00 mov $0x0,%dh
...
[ 35.457926][ T206] EAX: ffffffda EBX: 00000007 ECX: b7ea5bb1 EDX: 00000000
[ 35.458745][ T206] ESI: 01cdb258 EDI: 01cded88 EBP: 00000000 ESP: bfc6adf8
[ 35.459593][ T206] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b EFLAGS: 00000296
[ 35.460520][ T206] irq event stamp: 18793
[ 35.461019][ T206] hardirqs last enabled at (18801): __up_console_sem (printk.c:?)
[ 35.462038][ T206] hardirqs last disabled at (18808): __up_console_sem (printk.c:?)
[ 35.463061][ T206] softirqs last enabled at (0): copy_process (fork.c:?)
[ 35.464023][ T206] softirqs last disabled at (0): 0x0
[ 35.464773][ T206] ---[ end trace b87b4723e6116295 ]---
Starting LSB: Load kernel image with kexec...
[ OK ] Started LSB: Load kernel image with kexec.
LKP: HOSTNAME vm-snb-i386-129, MAC 36:ba:03:08:f1:da, kernel 5.16.0-06526-g64aee03f9836 1, serial console /dev/ttyS0
[ OK ] Listening on Load/Save RF Kill Switch Status /dev/rfkill Watch.
[ 36.524289][ T206] ------------[ cut here ]------------
[ 36.525031][ T206] CPA: called for zero pte. vaddr = f7493000 cpa->vaddr = f7491000
To reproduce:
# build kernel
cd linux
cp config-5.16.0-06526-g64aee03f9836 .config
make HOSTCC=clang-15 CC=clang-15 ARCH=i386 olddefconfig prepare modules_prepare bzImage modules
make HOSTCC=clang-15 CC=clang-15 ARCH=i386 INSTALL_MOD_PATH=<mod-install-dir> modules_install
cd <mod-install-dir>
find lib/ | cpio -o -H newc --quiet | gzip > modules.cgz
git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> -m modules.cgz job-script # job-script is attached in this email
# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.
---
0DAY/LKP+ Test Infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation
Thanks,
Oliver Sang