| Message ID | 7b5df1782e94a755b4a18733af44d17d8dd8b37b.1703149011.git.christophe.leroy@csgroup.eu (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] init: Declare rodata_enabled and mark_rodata_ro() at all time | expand |
Cc +Kees Christophe Leroy <christophe.leroy@csgroup.eu> writes: > Declaring rodata_enabled and mark_rodata_ro() at all time > helps removing related #ifdefery in C files. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > include/linux/init.h | 4 ---- > init/main.c | 21 +++++++-------------- > 2 files changed, 7 insertions(+), 18 deletions(-) > > diff --git a/include/linux/init.h b/include/linux/init.h > index 01b52c9c7526..d2b47be38a07 100644 > --- a/include/linux/init.h > +++ b/include/linux/init.h > @@ -168,12 +168,8 @@ extern initcall_entry_t __initcall_end[]; > > extern struct file_system_type rootfs_fs_type; > > -#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX) > extern bool rodata_enabled; > -#endif > -#ifdef CONFIG_STRICT_KERNEL_RWX > void mark_rodata_ro(void); > -#endif > > extern void (*late_time_init)(void); > > diff --git a/init/main.c b/init/main.c > index e24b0780fdff..807df08c501f 100644 > --- a/init/main.c > +++ b/init/main.c > @@ -1396,10 +1396,9 @@ static int __init set_debug_rodata(char *str) > early_param("rodata", set_debug_rodata); > #endif > > -#ifdef CONFIG_STRICT_KERNEL_RWX > static void mark_readonly(void) > { > - if (rodata_enabled) { > + if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) && rodata_enabled) { > /* > * load_module() results in W+X mappings, which are cleaned > * up with call_rcu(). Let's make sure that queued work is > @@ -1409,20 +1408,14 @@ static void mark_readonly(void) > rcu_barrier(); > mark_rodata_ro(); > rodata_test(); > - } else > + } else if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX)) { > pr_info("Kernel memory protection disabled.\n"); > + } else if (IS_ENABLED(CONFIG_ARCH_HAS_STRICT_KERNEL_RWX)) { > + pr_warn("Kernel memory protection not selected by kernel config.\n"); > + } else { > + pr_warn("This architecture does not have kernel memory protection.\n"); > + } > } > -#elif defined(CONFIG_ARCH_HAS_STRICT_KERNEL_RWX) > -static inline void mark_readonly(void) > -{ > - pr_warn("Kernel memory protection not selected by kernel config.\n"); > -} > -#else > -static inline void mark_readonly(void) > -{ > - pr_warn("This architecture does not have kernel memory protection.\n"); > -} > -#endif > > void __weak free_initmem(void) > { > -- > 2.41.0
On December 21, 2023 4:16:56 AM PST, Michael Ellerman <mpe@ellerman.id.au> wrote: >Cc +Kees > >Christophe Leroy <christophe.leroy@csgroup.eu> writes: >> Declaring rodata_enabled and mark_rodata_ro() at all time >> helps removing related #ifdefery in C files. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> include/linux/init.h | 4 ---- >> init/main.c | 21 +++++++-------------- >> 2 files changed, 7 insertions(+), 18 deletions(-) >> >> diff --git a/include/linux/init.h b/include/linux/init.h >> index 01b52c9c7526..d2b47be38a07 100644 >> --- a/include/linux/init.h >> +++ b/include/linux/init.h >> @@ -168,12 +168,8 @@ extern initcall_entry_t __initcall_end[]; >> >> extern struct file_system_type rootfs_fs_type; >> >> -#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX) >> extern bool rodata_enabled; >> -#endif >> -#ifdef CONFIG_STRICT_KERNEL_RWX >> void mark_rodata_ro(void); >> -#endif >> >> extern void (*late_time_init)(void); >> >> diff --git a/init/main.c b/init/main.c >> index e24b0780fdff..807df08c501f 100644 >> --- a/init/main.c >> +++ b/init/main.c >> @@ -1396,10 +1396,9 @@ static int __init set_debug_rodata(char *str) >> early_param("rodata", set_debug_rodata); >> #endif >> >> -#ifdef CONFIG_STRICT_KERNEL_RWX >> static void mark_readonly(void) >> { >> - if (rodata_enabled) { >> + if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) && rodata_enabled) { I think this will break without rodata_enabled actual existing on other architectures. (Only declaration was made visible, not the definition, which is above here and still behind ifdefs?) -Kees >> /* >> * load_module() results in W+X mappings, which are cleaned >> * up with call_rcu(). Let's make sure that queued work is >> @@ -1409,20 +1408,14 @@ static void mark_readonly(void) >> rcu_barrier(); >> mark_rodata_ro(); >> rodata_test(); >> - } else >> + } else if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX)) { >> pr_info("Kernel memory protection disabled.\n"); >> + } else if (IS_ENABLED(CONFIG_ARCH_HAS_STRICT_KERNEL_RWX)) { >> + pr_warn("Kernel memory protection not selected by kernel config.\n"); >> + } else { >> + pr_warn("This architecture does not have kernel memory protection.\n"); >> + } >> } >> -#elif defined(CONFIG_ARCH_HAS_STRICT_KERNEL_RWX) >> -static inline void mark_readonly(void) >> -{ >> - pr_warn("Kernel memory protection not selected by kernel config.\n"); >> -} >> -#else >> -static inline void mark_readonly(void) >> -{ >> - pr_warn("This architecture does not have kernel memory protection.\n"); >> -} >> -#endif >> >> void __weak free_initmem(void) >> { >> -- >> 2.41.0
Le 22/12/2023 à 06:35, Kees Cook a écrit : > [Vous ne recevez pas souvent de courriers de kees@kernel.org. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] > > On December 21, 2023 4:16:56 AM PST, Michael Ellerman <mpe@ellerman.id.au> wrote: >> Cc +Kees >> >> Christophe Leroy <christophe.leroy@csgroup.eu> writes: >>> Declaring rodata_enabled and mark_rodata_ro() at all time >>> helps removing related #ifdefery in C files. >>> >>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >>> --- >>> include/linux/init.h | 4 ---- >>> init/main.c | 21 +++++++-------------- >>> 2 files changed, 7 insertions(+), 18 deletions(-) >>> >>> diff --git a/include/linux/init.h b/include/linux/init.h >>> index 01b52c9c7526..d2b47be38a07 100644 >>> --- a/include/linux/init.h >>> +++ b/include/linux/init.h >>> @@ -168,12 +168,8 @@ extern initcall_entry_t __initcall_end[]; >>> >>> extern struct file_system_type rootfs_fs_type; >>> >>> -#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX) >>> extern bool rodata_enabled; >>> -#endif >>> -#ifdef CONFIG_STRICT_KERNEL_RWX >>> void mark_rodata_ro(void); >>> -#endif >>> >>> extern void (*late_time_init)(void); >>> >>> diff --git a/init/main.c b/init/main.c >>> index e24b0780fdff..807df08c501f 100644 >>> --- a/init/main.c >>> +++ b/init/main.c >>> @@ -1396,10 +1396,9 @@ static int __init set_debug_rodata(char *str) >>> early_param("rodata", set_debug_rodata); >>> #endif >>> >>> -#ifdef CONFIG_STRICT_KERNEL_RWX >>> static void mark_readonly(void) >>> { >>> - if (rodata_enabled) { >>> + if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) && rodata_enabled) { > > I think this will break without rodata_enabled actual existing on other architectures. (Only declaration was made visible, not the definition, which is above here and still behind ifdefs?) The compiler constant-folds IS_ENABLED(CONFIG_STRICT_KERNEL_RWX). When it is false, the second part is dropped. Exemple: bool test(void) { if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) && rodata_enabled) return true; else return false; } With CONFIG_STRICT_KERNEL_RWX set, it directly returns the content of rodata_enabled: 00000160 <test>: 160: 3d 20 00 00 lis r9,0 162: R_PPC_ADDR16_HA rodata_enabled 164: 88 69 00 00 lbz r3,0(r9) 166: R_PPC_ADDR16_LO rodata_enabled 168: 4e 80 00 20 blr With CONFIG_STRICT_KERNEL_RWX unset, it returns 0 and doesn't reference rodata_enabled at all: 000000bc <test>: bc: 38 60 00 00 li r3,0 c0: 4e 80 00 20 blr Many places in the kernel use this approach to minimise amount of #ifdefs. Christophe
On Thu, Dec 21, 2023 at 10:02:46AM +0100, Christophe Leroy wrote: > Declaring rodata_enabled and mark_rodata_ro() at all time > helps removing related #ifdefery in C files. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> Very nice cleanup, thanks!, applied and pushed Luis
Hi, On Mon, Jan 29, 2024 at 12:09:50PM -0800, Luis Chamberlain wrote: > On Thu, Dec 21, 2023 at 10:02:46AM +0100, Christophe Leroy wrote: > > Declaring rodata_enabled and mark_rodata_ro() at all time > > helps removing related #ifdefery in C files. > > > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > > Very nice cleanup, thanks!, applied and pushed > > Luis On next-20240130, which has your modules-next branch, and thus this series and the other "module: Use set_memory_rox()" series applied, my kernel crashes in some very weird way. Reverting your branch makes the crash go away. I thought I'd report it right away. Maybe you folks would know what's happening here? This is on arm64. [ 10.481015] Unable to handle kernel paging request at virtual address ffffffde85245d30 [ 10.490369] KASAN: maybe wild-memory-access in range [0x000000f42922e980-0x000000f42922e987] [ 10.503744] Mem abort info: [ 10.509383] ESR = 0x0000000096000047 [ 10.514400] EC = 0x25: DABT (current EL), IL = 32 bits [ 10.522366] SET = 0, FnV = 0 [ 10.526343] EA = 0, S1PTW = 0 [ 10.530695] FSC = 0x07: level 3 translation fault [ 10.537081] Data abort info: [ 10.540839] ISV = 0, ISS = 0x00000047, ISS2 = 0x00000000 [ 10.546456] CM = 0, WnR = 1, TnD = 0, TagAccess = 0 [ 10.551726] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 [ 10.557612] swapper pgtable: 4k pages, 39-bit VAs, pgdp=0000000041f98000 [ 10.565214] [ffffffde85245d30] pgd=100000023ffff003, p4d=100000023ffff003, pud=100000023ffff003, pmd=10000001121eb003, pte=0000000000000000 [ 10.578887] Internal error: Oops: 0000000096000047 [#1] PREEMPT SMP [ 10.585815] Modules linked in: [ 10.590235] CPU: 6 PID: 195 Comm: (udev-worker) Tainted: G B 6.8.0-rc2-next-20240130-02908-ge8ad01d60927-dirty #163 3f2318148ecc5fa70d1092c2b874f9b59bdb7d60 [ 10.607021] Hardware name: Google Tentacruel board (DT) [ 10.613607] pstate: a0400009 (NzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 10.621954] pc : module_bug_finalize+0x118/0x148 [ 10.626823] lr : module_bug_finalize+0x118/0x148 [ 10.631463] sp : ffffffc0820478d0 [ 10.631466] x29: ffffffc0820478d0 x28: ffffffc082047ca0 x27: ffffffde8d7d31a0 [ 10.631477] x26: ffffffde85223780 x25: 0000000000000000 x24: ffffffde8c413cc0 [ 10.631486] x23: ffffffde8dfcec80 x22: ffffffde8dfce000 x21: ffffffde85223ba8 [ 10.631495] x20: ffffffde85223780 x19: ffffffde85245d28 x18: 0000000000000000 [ 10.631504] x17: ffffffde8aa15938 x16: ffffffde8aabdd90 x15: ffffffde8aab8124 [ 10.631513] x14: ffffffde8acdd380 x13: 0000000041b58ab3 x12: ffffffbbd1bf9d91 [ 10.631522] x11: 1ffffffbd1bf9d90 x10: ffffffbbd1bf9d90 x9 : dfffffc000000000 [ 10.631531] x8 : 000000442e406270 x7 : ffffffde8dfcec87 x6 : 0000000000000001 [ 10.631539] x5 : ffffffde8dfcec80 x4 : 0000000000000000 x3 : ffffffde8bbadf08 [ 10.631548] x2 : 0000000000000001 x1 : ffffffde8eaff080 x0 : 0000000000000000 [ 10.631556] Call trace: [ 10.631559] module_bug_finalize+0x118/0x148 [ 10.631565] load_module+0x25ec/0x2a78 [ 10.631572] __do_sys_init_module+0x234/0x418 [ 10.631578] __arm64_sys_init_module+0x4c/0x68 [ 10.631584] invoke_syscall+0x68/0x198 [ 10.631589] el0_svc_common.constprop.0+0x11c/0x150 [ 10.631594] do_el0_svc+0x38/0x50 [ 10.631598] el0_svc+0x50/0xa0 [ 10.631604] el0t_64_sync_handler+0x120/0x130 [ 10.631609] el0t_64_sync+0x1a8/0x1b0 [ 10.631619] Code: 97c5418e c89ffef5 91002260 97c53ca7 (f9000675) [ 10.631624] ---[ end trace 0000000000000000 ]--- [ 10.642965] Kernel panic - not syncing: Oops: Fatal exception [ 10.642975] SMP: stopping secondary CPUs [ 10.648339] Kernel Offset: 0x1e0a800000 from 0xffffffc080000000 [ 10.648343] PHYS_OFFSET: 0x40000000 [ 10.648345] CPU features: 0x0,c0000061,7002814a,2100720b [ 10.648350] Memory Limit: none
Le 30/01/2024 à 10:16, Chen-Yu Tsai a écrit : > [Vous ne recevez pas souvent de courriers de wenst@chromium.org. D?couvrez pourquoi ceci est important ? https://aka.ms/LearnAboutSenderIdentification ] > > Hi, > > On Mon, Jan 29, 2024 at 12:09:50PM -0800, Luis Chamberlain wrote: >> On Thu, Dec 21, 2023 at 10:02:46AM +0100, Christophe Leroy wrote: >>> Declaring rodata_enabled and mark_rodata_ro() at all time >>> helps removing related #ifdefery in C files. >>> >>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> >> Very nice cleanup, thanks!, applied and pushed >> >> Luis > > On next-20240130, which has your modules-next branch, and thus this > series and the other "module: Use set_memory_rox()" series applied, > my kernel crashes in some very weird way. Reverting your branch > makes the crash go away. > > I thought I'd report it right away. Maybe you folks would know what's > happening here? This is on arm64. That's strange, it seems to bug in module_bug_finalize() which is _before_ calls to module_enable_ro() and such. Can you try to revert the 6 patches one by one to see which one introduces the problem ? In reality, only patch 677bfb9db8a3 really change things. Other ones are more on less only cleanup. Thanks Christophe > > [ 10.481015] Unable to handle kernel paging request at virtual address ffffffde85245d30 > [ 10.490369] KASAN: maybe wild-memory-access in range [0x000000f42922e980-0x000000f42922e987] > [ 10.503744] Mem abort info: > [ 10.509383] ESR = 0x0000000096000047 > [ 10.514400] EC = 0x25: DABT (current EL), IL = 32 bits > [ 10.522366] SET = 0, FnV = 0 > [ 10.526343] EA = 0, S1PTW = 0 > [ 10.530695] FSC = 0x07: level 3 translation fault > [ 10.537081] Data abort info: > [ 10.540839] ISV = 0, ISS = 0x00000047, ISS2 = 0x00000000 > [ 10.546456] CM = 0, WnR = 1, TnD = 0, TagAccess = 0 > [ 10.551726] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 > [ 10.557612] swapper pgtable: 4k pages, 39-bit VAs, pgdp=0000000041f98000 > [ 10.565214] [ffffffde85245d30] pgd=100000023ffff003, p4d=100000023ffff003, pud=100000023ffff003, pmd=10000001121eb003, pte=0000000000000000 > [ 10.578887] Internal error: Oops: 0000000096000047 [#1] PREEMPT SMP > [ 10.585815] Modules linked in: > [ 10.590235] CPU: 6 PID: 195 Comm: (udev-worker) Tainted: G B 6.8.0-rc2-next-20240130-02908-ge8ad01d60927-dirty #163 3f2318148ecc5fa70d1092c2b874f9b59bdb7d60 > [ 10.607021] Hardware name: Google Tentacruel board (DT) > [ 10.613607] pstate: a0400009 (NzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > [ 10.621954] pc : module_bug_finalize+0x118/0x148 > [ 10.626823] lr : module_bug_finalize+0x118/0x148 > [ 10.631463] sp : ffffffc0820478d0 > [ 10.631466] x29: ffffffc0820478d0 x28: ffffffc082047ca0 x27: ffffffde8d7d31a0 > [ 10.631477] x26: ffffffde85223780 x25: 0000000000000000 x24: ffffffde8c413cc0 > [ 10.631486] x23: ffffffde8dfcec80 x22: ffffffde8dfce000 x21: ffffffde85223ba8 > [ 10.631495] x20: ffffffde85223780 x19: ffffffde85245d28 x18: 0000000000000000 > [ 10.631504] x17: ffffffde8aa15938 x16: ffffffde8aabdd90 x15: ffffffde8aab8124 > [ 10.631513] x14: ffffffde8acdd380 x13: 0000000041b58ab3 x12: ffffffbbd1bf9d91 > [ 10.631522] x11: 1ffffffbd1bf9d90 x10: ffffffbbd1bf9d90 x9 : dfffffc000000000 > [ 10.631531] x8 : 000000442e406270 x7 : ffffffde8dfcec87 x6 : 0000000000000001 > [ 10.631539] x5 : ffffffde8dfcec80 x4 : 0000000000000000 x3 : ffffffde8bbadf08 > [ 10.631548] x2 : 0000000000000001 x1 : ffffffde8eaff080 x0 : 0000000000000000 > [ 10.631556] Call trace: > [ 10.631559] module_bug_finalize+0x118/0x148 > [ 10.631565] load_module+0x25ec/0x2a78 > [ 10.631572] __do_sys_init_module+0x234/0x418 > [ 10.631578] __arm64_sys_init_module+0x4c/0x68 > [ 10.631584] invoke_syscall+0x68/0x198 > [ 10.631589] el0_svc_common.constprop.0+0x11c/0x150 > [ 10.631594] do_el0_svc+0x38/0x50 > [ 10.631598] el0_svc+0x50/0xa0 > [ 10.631604] el0t_64_sync_handler+0x120/0x130 > [ 10.631609] el0t_64_sync+0x1a8/0x1b0 > [ 10.631619] Code: 97c5418e c89ffef5 91002260 97c53ca7 (f9000675) > [ 10.631624] ---[ end trace 0000000000000000 ]--- > [ 10.642965] Kernel panic - not syncing: Oops: Fatal exception > [ 10.642975] SMP: stopping secondary CPUs > [ 10.648339] Kernel Offset: 0x1e0a800000 from 0xffffffc080000000 > [ 10.648343] PHYS_OFFSET: 0x40000000 > [ 10.648345] CPU features: 0x0,c0000061,7002814a,2100720b > [ 10.648350] Memory Limit: none >
Dear All, On 30.01.2024 12:03, Christophe Leroy wrote: > Le 30/01/2024 à 10:16, Chen-Yu Tsai a écrit : >> [Vous ne recevez pas souvent de courriers de wenst@chromium.org. D?couvrez pourquoi ceci est important ? https://aka.ms/LearnAboutSenderIdentification ] >> >> On Mon, Jan 29, 2024 at 12:09:50PM -0800, Luis Chamberlain wrote: >>> On Thu, Dec 21, 2023 at 10:02:46AM +0100, Christophe Leroy wrote: >>>> Declaring rodata_enabled and mark_rodata_ro() at all time >>>> helps removing related #ifdefery in C files. >>>> >>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >>> Very nice cleanup, thanks!, applied and pushed >>> >>> Luis >> On next-20240130, which has your modules-next branch, and thus this >> series and the other "module: Use set_memory_rox()" series applied, >> my kernel crashes in some very weird way. Reverting your branch >> makes the crash go away. >> >> I thought I'd report it right away. Maybe you folks would know what's >> happening here? This is on arm64. > That's strange, it seems to bug in module_bug_finalize() which is > _before_ calls to module_enable_ro() and such. > > Can you try to revert the 6 patches one by one to see which one > introduces the problem ? > > In reality, only patch 677bfb9db8a3 really change things. Other ones are > more on less only cleanup. I've also run into this issue with today's (20240130) linux-next on my test farm. The issue is not fully reproducible, so it was a bit hard to bisect it automatically. I've spent some time on manual testing and it looks that reverting the following 2 commits on top of linux-next fixes the problem: 65929884f868 ("modules: Remove #ifdef CONFIG_STRICT_MODULE_RWX around rodata_enabled") 677bfb9db8a3 ("module: Don't ignore errors from set_memory_XX()") This in fact means that commit 677bfb9db8a3 is responsible for this regression, as 65929884f868 has to be reverted only because the latter depends on it. Let me know what I can do to help debugging this issue. Here is the stack trace I've got on Khadas VIM3 ARM64 board: Unable to handle kernel paging request at virtual address ffff80007bfeeb30 Mem abort info: ESR = 0x0000000096000047 EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 FSC = 0x07: level 3 translation fault Data abort info: ISV = 0, ISS = 0x00000047, ISS2 = 0x00000000 CM = 0, WnR = 1, TnD = 0, TagAccess = 0 GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 swapper pgtable: 4k pages, 48-bit VAs, pgdp=000000000a35a000 [ffff80007bfeeb30] pgd=10000000f4806003, p4d=10000000f4806003, pud=1000000007ed1003, pmd=1000000007ed2003, pte=0000000000000000 Internal error: Oops: 0000000096000047 [#1] PREEMPT SMP Modules linked in: CPU: 4 PID: 182 Comm: (udev-worker) Not tainted 6.8.0-rc2-next-20240130 #14391 Hardware name: Khadas VIM3 (DT) pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : module_bug_finalize+0xb0/0xdc lr : module_bug_finalize+0x70/0xdc ... Call trace: module_bug_finalize+0xb0/0xdc load_module+0x182c/0x1c88 init_module_from_file+0x84/0xc0 idempotent_init_module+0x180/0x250 __arm64_sys_finit_module+0x64/0xa0 invoke_syscall+0x48/0x114 el0_svc_common.constprop.0+0xc0/0xe0 do_el0_svc+0x1c/0x28 el0_svc+0x4c/0xe4 el0t_64_sync_handler+0xc0/0xc4 el0t_64_sync+0x190/0x194 Code: 9116e003 f942dc01 a93e8c41 c89ffc73 (f9000433) ---[ end trace 0000000000000000 ]--- Best regards
On Tue, Jan 30, 2024 at 06:48:11PM +0100, Marek Szyprowski wrote: > Dear All, > > On 30.01.2024 12:03, Christophe Leroy wrote: > > Le 30/01/2024 à 10:16, Chen-Yu Tsai a écrit : > >> [Vous ne recevez pas souvent de courriers de wenst@chromium.org. D?couvrez pourquoi ceci est important ? https://aka.ms/LearnAboutSenderIdentification ] > >> > >> On Mon, Jan 29, 2024 at 12:09:50PM -0800, Luis Chamberlain wrote: > >>> On Thu, Dec 21, 2023 at 10:02:46AM +0100, Christophe Leroy wrote: > >>>> Declaring rodata_enabled and mark_rodata_ro() at all time > >>>> helps removing related #ifdefery in C files. > >>>> > >>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > >>> Very nice cleanup, thanks!, applied and pushed > >>> > >>> Luis > >> On next-20240130, which has your modules-next branch, and thus this > >> series and the other "module: Use set_memory_rox()" series applied, > >> my kernel crashes in some very weird way. Reverting your branch > >> makes the crash go away. > >> > >> I thought I'd report it right away. Maybe you folks would know what's > >> happening here? This is on arm64. > > That's strange, it seems to bug in module_bug_finalize() which is > > _before_ calls to module_enable_ro() and such. > > > > Can you try to revert the 6 patches one by one to see which one > > introduces the problem ? > > > > In reality, only patch 677bfb9db8a3 really change things. Other ones are > > more on less only cleanup. > > I've also run into this issue with today's (20240130) linux-next on my > test farm. The issue is not fully reproducible, so it was a bit hard to > bisect it automatically. I've spent some time on manual testing and it > looks that reverting the following 2 commits on top of linux-next fixes > the problem: > > 65929884f868 ("modules: Remove #ifdef CONFIG_STRICT_MODULE_RWX around > rodata_enabled") > 677bfb9db8a3 ("module: Don't ignore errors from set_memory_XX()") > > This in fact means that commit 677bfb9db8a3 is responsible for this > regression, as 65929884f868 has to be reverted only because the latter > depends on it. Let me know what I can do to help debugging this issue. Thanks for the bisect, I've reset my tree to commit 3559ad395bf02 ("module: Change module_enable_{nx/x/ro}() to more explicit names") for now then, so to remove those commits. Luis
Le 30/01/2024 à 21:27, Luis Chamberlain a écrit : > On Tue, Jan 30, 2024 at 06:48:11PM +0100, Marek Szyprowski wrote: >> Dear All, >> >> On 30.01.2024 12:03, Christophe Leroy wrote: >>> Le 30/01/2024 à 10:16, Chen-Yu Tsai a écrit : >>>> [Vous ne recevez pas souvent de courriers de wenst@chromium.org. D?couvrez pourquoi ceci est important ? https://aka.ms/LearnAboutSenderIdentification ] >>>> >>>> On Mon, Jan 29, 2024 at 12:09:50PM -0800, Luis Chamberlain wrote: >>>>> On Thu, Dec 21, 2023 at 10:02:46AM +0100, Christophe Leroy wrote: >>>>>> Declaring rodata_enabled and mark_rodata_ro() at all time >>>>>> helps removing related #ifdefery in C files. >>>>>> >>>>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >>>>> Very nice cleanup, thanks!, applied and pushed >>>>> >>>>> Luis >>>> On next-20240130, which has your modules-next branch, and thus this >>>> series and the other "module: Use set_memory_rox()" series applied, >>>> my kernel crashes in some very weird way. Reverting your branch >>>> makes the crash go away. >>>> >>>> I thought I'd report it right away. Maybe you folks would know what's >>>> happening here? This is on arm64. >>> That's strange, it seems to bug in module_bug_finalize() which is >>> _before_ calls to module_enable_ro() and such. >>> >>> Can you try to revert the 6 patches one by one to see which one >>> introduces the problem ? >>> >>> In reality, only patch 677bfb9db8a3 really change things. Other ones are >>> more on less only cleanup. >> >> I've also run into this issue with today's (20240130) linux-next on my >> test farm. The issue is not fully reproducible, so it was a bit hard to >> bisect it automatically. I've spent some time on manual testing and it >> looks that reverting the following 2 commits on top of linux-next fixes >> the problem: >> >> 65929884f868 ("modules: Remove #ifdef CONFIG_STRICT_MODULE_RWX around >> rodata_enabled") >> 677bfb9db8a3 ("module: Don't ignore errors from set_memory_XX()") >> >> This in fact means that commit 677bfb9db8a3 is responsible for this >> regression, as 65929884f868 has to be reverted only because the latter >> depends on it. Let me know what I can do to help debugging this issue. > > Thanks for the bisect, I've reset my tree to commit > 3559ad395bf02 ("module: Change module_enable_{nx/x/ro}() to more > explicit names") for now then, so to remove those commits. > The problem being identified in commit 677bfb9db8a3 ("module: Don't ignore errors from set_memory_XX()"), you can keep/re-apply the series [PATCH 1/3] init: Declare rodata_enabled and mark_rodata_ro() at all time. Christophe
Hi, Le 30/01/2024 à 18:48, Marek Szyprowski a écrit : > [Vous ne recevez pas souvent de courriers de m.szyprowski@samsung.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] > > Dear All, > > On 30.01.2024 12:03, Christophe Leroy wrote: >> Le 30/01/2024 à 10:16, Chen-Yu Tsai a écrit : >>> [Vous ne recevez pas souvent de courriers de wenst@chromium.org. D?couvrez pourquoi ceci est important ? https://aka.ms/LearnAboutSenderIdentification ] >>> >>> On Mon, Jan 29, 2024 at 12:09:50PM -0800, Luis Chamberlain wrote: >>>> On Thu, Dec 21, 2023 at 10:02:46AM +0100, Christophe Leroy wrote: >>>>> Declaring rodata_enabled and mark_rodata_ro() at all time >>>>> helps removing related #ifdefery in C files. >>>>> >>>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >>>> Very nice cleanup, thanks!, applied and pushed >>>> >>>> Luis >>> On next-20240130, which has your modules-next branch, and thus this >>> series and the other "module: Use set_memory_rox()" series applied, >>> my kernel crashes in some very weird way. Reverting your branch >>> makes the crash go away. >>> >>> I thought I'd report it right away. Maybe you folks would know what's >>> happening here? This is on arm64. >> That's strange, it seems to bug in module_bug_finalize() which is >> _before_ calls to module_enable_ro() and such. >> >> Can you try to revert the 6 patches one by one to see which one >> introduces the problem ? >> >> In reality, only patch 677bfb9db8a3 really change things. Other ones are >> more on less only cleanup. > > I've also run into this issue with today's (20240130) linux-next on my > test farm. The issue is not fully reproducible, so it was a bit hard to > bisect it automatically. I've spent some time on manual testing and it > looks that reverting the following 2 commits on top of linux-next fixes > the problem: > > 65929884f868 ("modules: Remove #ifdef CONFIG_STRICT_MODULE_RWX around > rodata_enabled") > 677bfb9db8a3 ("module: Don't ignore errors from set_memory_XX()") > > This in fact means that commit 677bfb9db8a3 is responsible for this > regression, as 65929884f868 has to be reverted only because the latter > depends on it. Let me know what I can do to help debugging this issue. > Thanks for the bisect. I suspect you hit one of the errors and something goes wrong in the error path. To confirm this assumption, could you try with the following change on top of everything ? diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c index a14df9655dbe..fdf8484154dd 100644 --- a/kernel/module/strict_rwx.c +++ b/kernel/module/strict_rwx.c @@ -15,9 +15,12 @@ static int module_set_memory(const struct module *mod, enum mod_mem_type type, int (*set_memory)(unsigned long start, int num_pages)) { const struct module_memory *mod_mem = &mod->mem[type]; + int err; set_vm_flush_reset_perms(mod_mem->base); - return set_memory((unsigned long)mod_mem->base, mod_mem->size >> PAGE_SHIFT); + err = set_memory((unsigned long)mod_mem->base, mod_mem->size >> PAGE_SHIFT); + WARN(err, "module_set_memory(%d, %px, %x) returned %d\n", type, mod_mem->base, mod_mem->size, err); + return err; } /* Thanks for your help Christophe
Hi Christophe, On 31.01.2024 12:58, Christophe Leroy wrote: > Le 30/01/2024 à 18:48, Marek Szyprowski a écrit : >> [Vous ne recevez pas souvent de courriers de m.szyprowski@samsung.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] >> >> On 30.01.2024 12:03, Christophe Leroy wrote: >>> Le 30/01/2024 à 10:16, Chen-Yu Tsai a écrit : >>>> [Vous ne recevez pas souvent de courriers de wenst@chromium.org. D?couvrez pourquoi ceci est important ? https://aka.ms/LearnAboutSenderIdentification ] >>>> >>>> On Mon, Jan 29, 2024 at 12:09:50PM -0800, Luis Chamberlain wrote: >>>>> On Thu, Dec 21, 2023 at 10:02:46AM +0100, Christophe Leroy wrote: >>>>>> Declaring rodata_enabled and mark_rodata_ro() at all time >>>>>> helps removing related #ifdefery in C files. >>>>>> >>>>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >>>>> Very nice cleanup, thanks!, applied and pushed >>>>> >>>>> Luis >>>> On next-20240130, which has your modules-next branch, and thus this >>>> series and the other "module: Use set_memory_rox()" series applied, >>>> my kernel crashes in some very weird way. Reverting your branch >>>> makes the crash go away. >>>> >>>> I thought I'd report it right away. Maybe you folks would know what's >>>> happening here? This is on arm64. >>> That's strange, it seems to bug in module_bug_finalize() which is >>> _before_ calls to module_enable_ro() and such. >>> >>> Can you try to revert the 6 patches one by one to see which one >>> introduces the problem ? >>> >>> In reality, only patch 677bfb9db8a3 really change things. Other ones are >>> more on less only cleanup. >> I've also run into this issue with today's (20240130) linux-next on my >> test farm. The issue is not fully reproducible, so it was a bit hard to >> bisect it automatically. I've spent some time on manual testing and it >> looks that reverting the following 2 commits on top of linux-next fixes >> the problem: >> >> 65929884f868 ("modules: Remove #ifdef CONFIG_STRICT_MODULE_RWX around >> rodata_enabled") >> 677bfb9db8a3 ("module: Don't ignore errors from set_memory_XX()") >> >> This in fact means that commit 677bfb9db8a3 is responsible for this >> regression, as 65929884f868 has to be reverted only because the latter >> depends on it. Let me know what I can do to help debugging this issue. >> > Thanks for the bisect. I suspect you hit one of the errors and something > goes wrong in the error path. > > To confirm this assumption, could you try with the following change on > top of everything ? Yes, this is the problem. I've added printing a mod->name to the log. Here is a log from kernel build from next-20240130 (sometimes it even boots to shell): # dmesg | grep module_set_memory [ 8.061525] module_set_memory(6, 0000000000000000, 0) name ipv6 returned -22 [ 8.067543] WARNING: CPU: 3 PID: 1 at kernel/module/strict_rwx.c:22 module_set_memory+0x9c/0xb8 [ 8.097821] pc : module_set_memory+0x9c/0xb8 [ 8.102068] lr : module_set_memory+0x9c/0xb8 [ 8.183101] module_set_memory+0x9c/0xb8 [ 8.472862] module_set_memory(6, 0000000000000000, 0) name x_tables returned -22 [ 8.479215] WARNING: CPU: 2 PID: 1 at kernel/module/strict_rwx.c:22 module_set_memory+0x9c/0xb8 [ 8.510978] pc : module_set_memory+0x9c/0xb8 [ 8.515225] lr : module_set_memory+0x9c/0xb8 [ 8.596259] module_set_memory+0x9c/0xb8 [ 10.529879] module_set_memory(6, 0000000000000000, 0) name dm_mod returned -22 [ 10.536087] WARNING: CPU: 3 PID: 127 at kernel/module/strict_rwx.c:22 module_set_memory+0x9c/0xb8 [ 10.568254] pc : module_set_memory+0x9c/0xb8 [ 10.572501] lr : module_set_memory+0x9c/0xb8 [ 10.653535] module_set_memory+0x9c/0xb8 [ 10.853177] module_set_memory(6, 0000000000000000, 0) name fuse returned -22 [ 10.859196] WARNING: CPU: 5 PID: 130 at kernel/module/strict_rwx.c:22 module_set_memory+0x9c/0xb8 [ 10.891382] pc : module_set_memory+0x9c/0xb8 [ 10.895629] lr : module_set_memory+0x9c/0xb8 [ 10.976663] module_set_memory+0x9c/0xb8 > diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c > index a14df9655dbe..fdf8484154dd 100644 > --- a/kernel/module/strict_rwx.c > +++ b/kernel/module/strict_rwx.c > @@ -15,9 +15,12 @@ static int module_set_memory(const struct module > *mod, enum mod_mem_type type, > int (*set_memory)(unsigned long start, int num_pages)) > { > const struct module_memory *mod_mem = &mod->mem[type]; > + int err; > > set_vm_flush_reset_perms(mod_mem->base); > - return set_memory((unsigned long)mod_mem->base, mod_mem->size >> > PAGE_SHIFT); > + err = set_memory((unsigned long)mod_mem->base, mod_mem->size >> > PAGE_SHIFT); > + WARN(err, "module_set_memory(%d, %px, %x) returned %d\n", type, > mod_mem->base, mod_mem->size, err); > + return err; > } > > /* > > > Thanks for your help > Christophe Best regards
Le 31/01/2024 à 16:17, Marek Szyprowski a écrit : > [Vous ne recevez pas souvent de courriers de m.szyprowski@samsung.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] > > Hi Christophe, > > On 31.01.2024 12:58, Christophe Leroy wrote: >> Le 30/01/2024 à 18:48, Marek Szyprowski a écrit : >>> [Vous ne recevez pas souvent de courriers de m.szyprowski@samsung.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] >>> >>> On 30.01.2024 12:03, Christophe Leroy wrote: >>>> Le 30/01/2024 à 10:16, Chen-Yu Tsai a écrit : >>>>> [Vous ne recevez pas souvent de courriers de wenst@chromium.org. D?couvrez pourquoi ceci est important ? https://aka.ms/LearnAboutSenderIdentification ] >>>>> >>>>> On Mon, Jan 29, 2024 at 12:09:50PM -0800, Luis Chamberlain wrote: >>>>>> On Thu, Dec 21, 2023 at 10:02:46AM +0100, Christophe Leroy wrote: >>>>>>> Declaring rodata_enabled and mark_rodata_ro() at all time >>>>>>> helps removing related #ifdefery in C files. >>>>>>> >>>>>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >>>>>> Very nice cleanup, thanks!, applied and pushed >>>>>> >>>>>> Luis >>>>> On next-20240130, which has your modules-next branch, and thus this >>>>> series and the other "module: Use set_memory_rox()" series applied, >>>>> my kernel crashes in some very weird way. Reverting your branch >>>>> makes the crash go away. >>>>> >>>>> I thought I'd report it right away. Maybe you folks would know what's >>>>> happening here? This is on arm64. >>>> That's strange, it seems to bug in module_bug_finalize() which is >>>> _before_ calls to module_enable_ro() and such. >>>> >>>> Can you try to revert the 6 patches one by one to see which one >>>> introduces the problem ? >>>> >>>> In reality, only patch 677bfb9db8a3 really change things. Other ones are >>>> more on less only cleanup. >>> I've also run into this issue with today's (20240130) linux-next on my >>> test farm. The issue is not fully reproducible, so it was a bit hard to >>> bisect it automatically. I've spent some time on manual testing and it >>> looks that reverting the following 2 commits on top of linux-next fixes >>> the problem: >>> >>> 65929884f868 ("modules: Remove #ifdef CONFIG_STRICT_MODULE_RWX around >>> rodata_enabled") >>> 677bfb9db8a3 ("module: Don't ignore errors from set_memory_XX()") >>> >>> This in fact means that commit 677bfb9db8a3 is responsible for this >>> regression, as 65929884f868 has to be reverted only because the latter >>> depends on it. Let me know what I can do to help debugging this issue. >>> >> Thanks for the bisect. I suspect you hit one of the errors and something >> goes wrong in the error path. >> >> To confirm this assumption, could you try with the following change on >> top of everything ? > > > Yes, this is the problem. I've added printing a mod->name to the log. > Here is a log from kernel build from next-20240130 (sometimes it even > boots to shell): > > # dmesg | grep module_set_memory > [ 8.061525] module_set_memory(6, 0000000000000000, 0) name ipv6 > returned -22 > [ 8.067543] WARNING: CPU: 3 PID: 1 at kernel/module/strict_rwx.c:22 > module_set_memory+0x9c/0xb8 Would be good if you could show the backtrace too so that we know who is the caller. I guess what you show here is what you get on the screen ? The backtrace should be available throught 'dmesg'. I guess we will now seek help from ARM64 people to understand why module_set_memory_something() fails with -EINVAL when loading modules. > [ 8.097821] pc : module_set_memory+0x9c/0xb8 > [ 8.102068] lr : module_set_memory+0x9c/0xb8 > [ 8.183101] module_set_memory+0x9c/0xb8 > [ 8.472862] module_set_memory(6, 0000000000000000, 0) name x_tables > returned -22 > [ 8.479215] WARNING: CPU: 2 PID: 1 at kernel/module/strict_rwx.c:22 > module_set_memory+0x9c/0xb8 > [ 8.510978] pc : module_set_memory+0x9c/0xb8 > [ 8.515225] lr : module_set_memory+0x9c/0xb8 > [ 8.596259] module_set_memory+0x9c/0xb8 > [ 10.529879] module_set_memory(6, 0000000000000000, 0) name dm_mod > returned -22 > [ 10.536087] WARNING: CPU: 3 PID: 127 at kernel/module/strict_rwx.c:22 > module_set_memory+0x9c/0xb8 > [ 10.568254] pc : module_set_memory+0x9c/0xb8 > [ 10.572501] lr : module_set_memory+0x9c/0xb8 > [ 10.653535] module_set_memory+0x9c/0xb8 > [ 10.853177] module_set_memory(6, 0000000000000000, 0) name fuse > returned -22 > [ 10.859196] WARNING: CPU: 5 PID: 130 at kernel/module/strict_rwx.c:22 > module_set_memory+0x9c/0xb8 > [ 10.891382] pc : module_set_memory+0x9c/0xb8 > [ 10.895629] lr : module_set_memory+0x9c/0xb8 > [ 10.976663] module_set_memory+0x9c/0xb8 > > > >> diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c >> index a14df9655dbe..fdf8484154dd 100644 >> --- a/kernel/module/strict_rwx.c >> +++ b/kernel/module/strict_rwx.c >> @@ -15,9 +15,12 @@ static int module_set_memory(const struct module >> *mod, enum mod_mem_type type, >> int (*set_memory)(unsigned long start, int num_pages)) >> { >> const struct module_memory *mod_mem = &mod->mem[type]; >> + int err; >> >> set_vm_flush_reset_perms(mod_mem->base); >> - return set_memory((unsigned long)mod_mem->base, mod_mem->size >> >> PAGE_SHIFT); >> + err = set_memory((unsigned long)mod_mem->base, mod_mem->size >> >> PAGE_SHIFT); >> + WARN(err, "module_set_memory(%d, %px, %x) returned %d\n", type, >> mod_mem->base, mod_mem->size, err); >> + return err; >> } >> >> /* >> >> >> Thanks for your help >> Christophe > > Best regards > -- > Marek Szyprowski, PhD > Samsung R&D Institute Poland >
Hi Christophe, On 31.01.2024 21:07, Christophe Leroy wrote: > Le 31/01/2024 à 16:17, Marek Szyprowski a écrit : >> [Vous ne recevez pas souvent de courriers de m.szyprowski@samsung.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] >> >> On 31.01.2024 12:58, Christophe Leroy wrote: >>> Le 30/01/2024 à 18:48, Marek Szyprowski a écrit : >>>> [Vous ne recevez pas souvent de courriers de m.szyprowski@samsung.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] >>>> >>>> On 30.01.2024 12:03, Christophe Leroy wrote: >>>>> Le 30/01/2024 à 10:16, Chen-Yu Tsai a écrit : >>>>>> [Vous ne recevez pas souvent de courriers de wenst@chromium.org. D?couvrez pourquoi ceci est important ? https://aka.ms/LearnAboutSenderIdentification ] >>>>>> >>>>>> On Mon, Jan 29, 2024 at 12:09:50PM -0800, Luis Chamberlain wrote: >>>>>>> On Thu, Dec 21, 2023 at 10:02:46AM +0100, Christophe Leroy wrote: >>>>>>>> Declaring rodata_enabled and mark_rodata_ro() at all time >>>>>>>> helps removing related #ifdefery in C files. >>>>>>>> >>>>>>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >>>>>>> Very nice cleanup, thanks!, applied and pushed >>>>>>> >>>>>>> Luis >>>>>> On next-20240130, which has your modules-next branch, and thus this >>>>>> series and the other "module: Use set_memory_rox()" series applied, >>>>>> my kernel crashes in some very weird way. Reverting your branch >>>>>> makes the crash go away. >>>>>> >>>>>> I thought I'd report it right away. Maybe you folks would know what's >>>>>> happening here? This is on arm64. >>>>> That's strange, it seems to bug in module_bug_finalize() which is >>>>> _before_ calls to module_enable_ro() and such. >>>>> >>>>> Can you try to revert the 6 patches one by one to see which one >>>>> introduces the problem ? >>>>> >>>>> In reality, only patch 677bfb9db8a3 really change things. Other ones are >>>>> more on less only cleanup. >>>> I've also run into this issue with today's (20240130) linux-next on my >>>> test farm. The issue is not fully reproducible, so it was a bit hard to >>>> bisect it automatically. I've spent some time on manual testing and it >>>> looks that reverting the following 2 commits on top of linux-next fixes >>>> the problem: >>>> >>>> 65929884f868 ("modules: Remove #ifdef CONFIG_STRICT_MODULE_RWX around >>>> rodata_enabled") >>>> 677bfb9db8a3 ("module: Don't ignore errors from set_memory_XX()") >>>> >>>> This in fact means that commit 677bfb9db8a3 is responsible for this >>>> regression, as 65929884f868 has to be reverted only because the latter >>>> depends on it. Let me know what I can do to help debugging this issue. >>>> >>> Thanks for the bisect. I suspect you hit one of the errors and something >>> goes wrong in the error path. >>> >>> To confirm this assumption, could you try with the following change on >>> top of everything ? >> >> Yes, this is the problem. I've added printing a mod->name to the log. >> Here is a log from kernel build from next-20240130 (sometimes it even >> boots to shell): >> >> # dmesg | grep module_set_memory >> [ 8.061525] module_set_memory(6, 0000000000000000, 0) name ipv6 >> returned -22 >> [ 8.067543] WARNING: CPU: 3 PID: 1 at kernel/module/strict_rwx.c:22 >> module_set_memory+0x9c/0xb8 > Would be good if you could show the backtrace too so that we know who is > the caller. I guess what you show here is what you get on the screen ? > The backtrace should be available throught 'dmesg'. Here are relevant parts of the boot log: [ 8.096850] ------------[ cut here ]------------ [ 8.096939] module_set_memory(6, 0000000000000000, 0) name ipv6 returned -22 [ 8.102947] WARNING: CPU: 4 PID: 1 at kernel/module/strict_rwx.c:22 module_set_memory+0x9c/0xb8 [ 8.111561] Modules linked in: [ 8.114596] CPU: 4 PID: 1 Comm: systemd Not tainted 6.8.0-rc2-next-20240130-dirty #14429 [ 8.122654] Hardware name: Khadas VIM3 (DT) [ 8.126815] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 8.133747] pc : module_set_memory+0x9c/0xb8 [ 8.137994] lr : module_set_memory+0x9c/0xb8 [ 8.142240] sp : ffff800083fcba80 [ 8.145534] x29: ffff800083fcba80 x28: 0000000000000001 x27: ffff80007c024448 [ 8.152640] x26: ffff800083fcbc10 x25: ffff80007c007958 x24: ffff80007c024450 [ 8.159747] x23: ffff800083f2a090 x22: ffff80007c007940 x21: 0000000000000006 [ 8.166854] x20: 00000000ffffffea x19: ffff80007c007af0 x18: 0000000000000030 [ 8.173960] x17: 0000000000000000 x16: 0000000000005932 x15: ffffffffffffffff [ 8.181067] x14: ffff800082ea5658 x13: 00000000000003d5 x12: 0000000000000147 [ 8.188174] x11: 6920656d616e2029 x10: ffff800082efd658 x9 : 00000000fffff000 [ 8.195280] x8 : ffff800082ea5658 x7 : ffff800082efd658 x6 : 0000000000000000 [ 8.202387] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000 [ 8.209494] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0000001f0000 [ 8.216601] Call trace: [ 8.219027] module_set_memory+0x9c/0xb8 [ 8.222927] module_enable_rodata_ro+0x64/0xa0 [ 8.227347] load_module+0x1838/0x1c88 [ 8.231074] init_module_from_file+0x84/0xc0 [ 8.235320] idempotent_init_module+0x180/0x250 [ 8.239827] __arm64_sys_finit_module+0x64/0xa0 [ 8.244334] invoke_syscall+0x48/0x114 [ 8.248060] el0_svc_common.constprop.0+0x40/0xe0 [ 8.252740] do_el0_svc+0x1c/0x28 [ 8.256034] el0_svc+0x4c/0xe4 [ 8.259067] el0t_64_sync_handler+0xc0/0xc4 [ 8.263227] el0t_64_sync+0x190/0x194 [ 8.266868] irq event stamp: 1304234 [ 8.270421] hardirqs last enabled at (1304233): [<ffff80008012c95c>] console_unlock+0x120/0x12c [ 8.279174] hardirqs last disabled at (1304234): [<ffff800081239bc0>] el1_dbg+0x24/0x8c [ 8.287147] softirqs last enabled at (1301556): [<ffff800080010a60>] __do_softirq+0x4a0/0x4e8 [ 8.295727] softirqs last disabled at (1301545): [<ffff8000800169b0>] ____do_softirq+0x10/0x1c [ 8.304307] ---[ end trace 0000000000000000 ]--- [ 8.508381] ------------[ cut here ]------------ [ 8.508432] module_set_memory(6, 0000000000000000, 0) name x_tables returned -22 [ 8.514785] WARNING: CPU: 2 PID: 1 at kernel/module/strict_rwx.c:22 module_set_memory+0x9c/0xb8 [ 8.523410] Modules linked in: [ 8.526444] CPU: 2 PID: 1 Comm: systemd Tainted: G W 6.8.0-rc2-next-20240130-dirty #14429 [ 8.535976] Hardware name: Khadas VIM3 (DT) [ 8.540137] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 8.547069] pc : module_set_memory+0x9c/0xb8 [ 8.551316] lr : module_set_memory+0x9c/0xb8 [ 8.555562] sp : ffff800083fcba80 [ 8.558856] x29: ffff800083fcba80 x28: 0000000000000001 x27: ffff80007bfab1f0 [ 8.565963] x26: ffff800083fcbc10 x25: ffff80007bfa5458 x24: ffff80007bfab1f8 [ 8.573069] x23: ffff800083f2a090 x22: ffff80007bfa5440 x21: 0000000000000006 [ 8.580176] x20: 00000000ffffffea x19: ffff80007bfa55f0 x18: 0000000000000030 [ 8.587282] x17: 64656e7275746572 x16: 2073656c6261745f x15: 7820656d616e2029 [ 8.594389] x14: ffff800082ea5658 x13: 000000000000044a x12: 000000000000016e [ 8.601496] x11: 6261745f7820656d x10: ffff800082efd658 x9 : 00000000fffff000 [ 8.608602] x8 : ffff800082ea5658 x7 : ffff800082efd658 x6 : 0000000000000000 [ 8.615709] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000 [ 8.622816] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0000001f0000 [ 8.629923] Call trace: [ 8.632349] module_set_memory+0x9c/0xb8 [ 8.636249] module_enable_rodata_ro+0x64/0xa0 [ 8.640669] load_module+0x1838/0x1c88 [ 8.644396] init_module_from_file+0x84/0xc0 [ 8.648642] idempotent_init_module+0x180/0x250 [ 8.653149] __arm64_sys_finit_module+0x64/0xa0 [ 8.657656] invoke_syscall+0x48/0x114 [ 8.661383] el0_svc_common.constprop.0+0x40/0xe0 [ 8.666062] do_el0_svc+0x1c/0x28 [ 8.669356] el0_svc+0x4c/0xe4 [ 8.672389] el0t_64_sync_handler+0xc0/0xc4 [ 8.676549] el0t_64_sync+0x190/0x194 [ 8.680190] irq event stamp: 1304890 [ 8.683742] hardirqs last enabled at (1304889): [<ffff80008012c95c>] console_unlock+0x120/0x12c [ 8.692496] hardirqs last disabled at (1304890): [<ffff800081239bc0>] el1_dbg+0x24/0x8c [ 8.700469] softirqs last enabled at (1304870): [<ffff800080010a60>] __do_softirq+0x4a0/0x4e8 [ 8.709049] softirqs last disabled at (1304865): [<ffff8000800169b0>] ____do_softirq+0x10/0x1c [ 8.717629] ---[ end trace 0000000000000000 ]--- [ 10.560872] ------------[ cut here ]------------ [ 10.560924] module_set_memory(6, 0000000000000000, 0) name dm_mod returned -22 [ 10.567128] WARNING: CPU: 4 PID: 127 at kernel/module/strict_rwx.c:22 module_set_memory+0x9c/0xb8 [ 10.575902] Modules linked in: [ 10.578937] CPU: 4 PID: 127 Comm: modprobe Tainted: G W 6.8.0-rc2-next-20240130-dirty #14429 [ 10.588728] Hardware name: Khadas VIM3 (DT) [ 10.592889] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 10.599821] pc : module_set_memory+0x9c/0xb8 [ 10.604068] lr : module_set_memory+0x9c/0xb8 [ 10.608314] sp : ffff800084da3a80 [ 10.611608] x29: ffff800084da3a80 x28: 0000000000000001 x27: ffff80007bfbf22c [ 10.618714] x26: ffff800084da3c10 x25: ffff80007bfb4c58 x24: ffff80007bfbf234 [ 10.623602] systemd[1]: Starting modprobe@efi_pstore.service - Load Kernel Module efi_pstore... [ 10.625820] x23: ffff800083f2a090 x22: ffff80007bfb4c40 x21: 0000000000000006 [ 10.625848] x20: 00000000ffffffea x19: ffff80007bfb4df0 x18: 0000000000000030 [ 10.648751] x17: 2d2064656e727574 x16: 657220646f6d5f6d x15: 6420656d616e2029 [ 10.648768] x14: ffff800082ea5658 x13: 000000000000051f x12: 00000000000001b5 [ 10.648782] x11: 5f6d6420656d616e x10: ffff800082efd658 x9 : 00000000fffff000 [ 10.648795] x8 : ffff800082ea5658 x7 : ffff800082efd658 x6 : 0000000000000000 [ 10.677128] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000 [ 10.684235] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff00003d341b80 [ 10.691342] Call trace: [ 10.693768] module_set_memory+0x9c/0xb8 [ 10.697668] module_enable_rodata_ro+0x64/0xa0 [ 10.702088] load_module+0x1838/0x1c88 [ 10.705815] init_module_from_file+0x84/0xc0 [ 10.710061] idempotent_init_module+0x180/0x250 [ 10.714568] __arm64_sys_finit_module+0x64/0xa0 [ 10.719075] invoke_syscall+0x48/0x114 [ 10.722801] el0_svc_common.constprop.0+0x40/0xe0 [ 10.727481] do_el0_svc+0x1c/0x28 [ 10.730774] el0_svc+0x4c/0xe4 [ 10.733808] el0t_64_sync_handler+0xc0/0xc4 [ 10.737968] el0t_64_sync+0x190/0x194 [ 10.741609] irq event stamp: 13696 [ 10.744988] hardirqs last enabled at (13695): [<ffff80008012c95c>] console_unlock+0x120/0x12c [ 10.753568] hardirqs last disabled at (13696): [<ffff800081239bc0>] el1_dbg+0x24/0x8c [ 10.761368] softirqs last enabled at (13692): [<ffff800080010a60>] __do_softirq+0x4a0/0x4e8 [ 10.769774] softirqs last disabled at (13683): [<ffff8000800169b0>] ____do_softirq+0x10/0x1c [ 10.778181] ---[ end trace 0000000000000000 ]--- [ 10.860938] ------------[ cut here ]------------ [ 10.860989] module_set_memory(6, 0000000000000000, 0) name fuse returned -22 [ 10.867007] WARNING: CPU: 3 PID: 130 at kernel/module/strict_rwx.c:22 module_set_memory+0x9c/0xb8 [ 10.875796] Modules linked in: [ 10.878829] CPU: 3 PID: 130 Comm: modprobe Tainted: G W 6.8.0-rc2-next-20240130-dirty #14429 [ 10.888621] Hardware name: Khadas VIM3 (DT) [ 10.892781] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 10.899714] pc : module_set_memory+0x9c/0xb8 [ 10.903960] lr : module_set_memory+0x9c/0xb8 [ 10.908207] sp : ffff800084d8ba80 [ 10.911500] x29: ffff800084d8ba80 x28: 0000000000000001 x27: ffff80007bff7148 [ 10.918607] x26: ffff800084d8bc10 x25: ffff80007bfee758 x24: ffff80007bff7150 [ 10.925714] x23: ffff800083f2a090 x22: ffff80007bfee740 x21: 0000000000000006 [ 10.932820] x20: 00000000ffffffea x19: ffff80007bfee8f0 x18: 0000000000000030 [ 10.939927] x17: 0000000000000000 x16: 0000000000000000 x15: ffffffffffffffff [ 10.947034] x14: ffff800082ea5658 x13: 000000000000059d x12: 00000000000001df [ 10.954140] x11: 6620656d616e2029 x10: ffff800082efd658 x9 : 00000000fffff000 [ 10.961247] x8 : ffff800082ea5658 x7 : ffff800082efd658 x6 : 0000000000000000 [ 10.968354] x5 : 000000000000bff4 x4 : 0000000000000000 x3 : 0000000000000000 [ 10.975460] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff00000268d280 [ 10.982568] Call trace: [ 10.984994] module_set_memory+0x9c/0xb8 [ 10.988894] module_enable_rodata_ro+0x64/0xa0 [ 10.993314] load_module+0x1838/0x1c88 [ 10.997041] init_module_from_file+0x84/0xc0 [ 11.001287] idempotent_init_module+0x180/0x250 [ 11.005794] __arm64_sys_finit_module+0x64/0xa0 [ 11.010301] invoke_syscall+0x48/0x114 [ 11.014027] el0_svc_common.constprop.0+0x40/0xe0 [ 11.018707] do_el0_svc+0x1c/0x28 [ 11.022000] el0_svc+0x4c/0xe4 [ 11.025034] el0t_64_sync_handler+0xc0/0xc4 [ 11.029194] el0t_64_sync+0x190/0x194 [ 11.032835] irq event stamp: 13612 [ 11.036214] hardirqs last enabled at (13611): [<ffff80008012c95c>] console_unlock+0x120/0x12c [ 11.044794] hardirqs last disabled at (13612): [<ffff800081239bc0>] el1_dbg+0x24/0x8c [ 11.052594] softirqs last enabled at (13584): [<ffff800080010a60>] __do_softirq+0x4a0/0x4e8 [ 11.061000] softirqs last disabled at (13333): [<ffff8000800169b0>] ____do_softirq+0x10/0x1c [ 11.069407] ---[ end trace 0000000000000000 ]--- > I guess we will now seek help from ARM64 people to understand why > module_set_memory_something() fails with -EINVAL when loading modules. > > ... Best regards
On Wed, Jan 31, 2024 at 06:53:13AM +0000, Christophe Leroy wrote: > The problem being identified in commit 677bfb9db8a3 ("module: Don't > ignore errors from set_memory_XX()"), you can keep/re-apply the series > [PATCH 1/3] init: Declare rodata_enabled and mark_rodata_ro() at all time. Sure, queued that up into modules-testing before I push to modules-next. Luis
diff --git a/include/linux/init.h b/include/linux/init.h index 01b52c9c7526..d2b47be38a07 100644 --- a/include/linux/init.h +++ b/include/linux/init.h @@ -168,12 +168,8 @@ extern initcall_entry_t __initcall_end[]; extern struct file_system_type rootfs_fs_type; -#if defined(CONFIG_STRICT_KERNEL_RWX) || defined(CONFIG_STRICT_MODULE_RWX) extern bool rodata_enabled; -#endif -#ifdef CONFIG_STRICT_KERNEL_RWX void mark_rodata_ro(void); -#endif extern void (*late_time_init)(void); diff --git a/init/main.c b/init/main.c index e24b0780fdff..807df08c501f 100644 --- a/init/main.c +++ b/init/main.c @@ -1396,10 +1396,9 @@ static int __init set_debug_rodata(char *str) early_param("rodata", set_debug_rodata); #endif -#ifdef CONFIG_STRICT_KERNEL_RWX static void mark_readonly(void) { - if (rodata_enabled) { + if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) && rodata_enabled) { /* * load_module() results in W+X mappings, which are cleaned * up with call_rcu(). Let's make sure that queued work is @@ -1409,20 +1408,14 @@ static void mark_readonly(void) rcu_barrier(); mark_rodata_ro(); rodata_test(); - } else + } else if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX)) { pr_info("Kernel memory protection disabled.\n"); + } else if (IS_ENABLED(CONFIG_ARCH_HAS_STRICT_KERNEL_RWX)) { + pr_warn("Kernel memory protection not selected by kernel config.\n"); + } else { + pr_warn("This architecture does not have kernel memory protection.\n"); + } } -#elif defined(CONFIG_ARCH_HAS_STRICT_KERNEL_RWX) -static inline void mark_readonly(void) -{ - pr_warn("Kernel memory protection not selected by kernel config.\n"); -} -#else -static inline void mark_readonly(void) -{ - pr_warn("This architecture does not have kernel memory protection.\n"); -} -#endif void __weak free_initmem(void) {
Declaring rodata_enabled and mark_rodata_ro() at all time helps removing related #ifdefery in C files. Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- include/linux/init.h | 4 ---- init/main.c | 21 +++++++-------------- 2 files changed, 7 insertions(+), 18 deletions(-)