Hi, 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 ] > > 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@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 ? 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
