Le 31/01/2024 à 16:17, Marek Szyprowski a écrit : > [Vous ne recevez pas souvent de courriers de m.szyprowski@xxxxxxxxxxx. 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@xxxxxxxxxxx. 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@xxxxxxxxxxxx. 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@xxxxxxxxxx> >>>>>> 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 >
