Hi Kazu,
On Fri, Nov 10, 2023 at 1:19 PM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab@xxxxxxx> wrote:
>
> On 2023/11/10 12:15, Tao Liu wrote:
>
> >> If a module already does not have its init memory range, it might be
> >> a bit better to not specify "-s .init.text <addr>" to add-symbol-file..
> >>
> >
> > Thanks a lot for finding the root cause. My patch is just a
> > work-around, and I think your "trial" patch is better to be applied. I
> > agree the ".init.text" should not be added by add-symbol-file, since
> > this section will be freed and will be occupied by other modules after
> > kernel init, which will cause symbols overlap. Could you please draft
> > the "trial" patch to be the formal one?
>
> The trial patch just doesn't add the .init.text unconditionally, but it
> should be required occasionally e.g. a panic when module initialization?
> As crash has a symbol table for a module init range:
>
> crash> help -s
> ...
> mod_base: ffffffffc0092000
> module_struct: ffffffffc009db00
> mod_name: floppy
> mod_size: 69417
> mod_namelist: /home/vmcore/symbol_err/...
> mod_flags: 2a (MOD_LOAD_SYMS|MOD_KALLSYMS|MOD_NOPATCH)
> mod_symtable: 6a28170
> mod_symend: 6a2a268
> mod_init_symtable: 0 <<<
> mod_init_symend: 0 <<<
>
>
> So to finish a patch, we will need to look into a few more things:
>
> - It looks like the .init.{text,data} sections are always added on that
> kernel version, whether this is intended or not. For example, they
> might be wrongly added due to a name change of a struct member.
>
> - If it's an intended behavior, how we can exclude the .init sections
> only when it's unnecessary.
>
I agree the .init.{text,data} sections should be loaded if the module
crashes during init.
Here is a trial patch which worked for me:
diff --git a/symbols.c b/symbols.c
index 8e8b4c3..24b879d 100644
--- a/symbols.c
+++ b/symbols.c
@@ -13096,6 +13096,7 @@ add_symbol_file_kallsyms(struct load_module
*lm, struct gnu_request *req)
char buf[BUFSIZE];
char section_name[BUFSIZE/2];
ulong section_vaddr;
+ long mod_init_module_ptr;
#if defined(GDB_5_3) || defined(GDB_6_0) || defined(GDB_6_1)
return FALSE;
@@ -13191,6 +13192,10 @@ add_symbol_file_kallsyms(struct load_module
*lm, struct gnu_request *req)
req->buf = GETBUF(buflen = 1024);
retval = FALSE;
+ readmem(lm->module_struct + MODULE_OFFSET2(module_module_init,
rx), KVADDR,
+ &mod_init_module_ptr, sizeof(void *), "module.module_init",
FAULT_ON_ERROR);
+ fprintf(fp, ">>>>>>>> %lx\n", mod_init_module_ptr);
+
for (done = FALSE; !done; array_entry += SIZE(module_sect_attr)) {
switch (st->flags & MODSECT_VMASK)
@@ -13283,7 +13288,7 @@ add_symbol_file_kallsyms(struct load_module
*lm, struct gnu_request *req)
shift_string_right(req->buf, strlen(buf));
BCOPY(buf, req->buf, strlen(buf));
retval = TRUE;
- } else {
+ } else if (mod_init_module_ptr ||
strncmp(section_name, ".init.", 6)) {
sprintf(buf, " -s %s 0x%lx", section_name,
section_vaddr);
while ((len + strlen(buf)) >= buflen) {
RESIZEBUF(req->buf, buflen, buflen * 2);
According to kernel source kernel/module.c:do_init_module, after
module been init successfully, module_init etc will be freed, and it
will be reset to NULL:
unset_module_init_ro_nx(mod);
module_free(mod, mod->module_init);
mod->module_init = NULL; <<----------
mod->init_size = 0;
mod->init_ro_size = 0;
mod->init_text_size = 0;
So in crash, if the module_init member of struct module is NULL, we
can be sure this module has been init successfully and the .init.
section has been freed, so there is no need to load .init. section by
add-symbol-file. However if we encounter the module_init with a value,
the .init. section is not freed and should be loaded by
add-symbol-file.
Here is a kernel module which will fail during init for testing:
hello/hello.c:
static int __init hello_world_init(void) {
int ret = 0;
printk(KERN_INFO "Hello, World!\n");
*(int *)ret = 100;
return 0; // Initialization successful
}
crash> mod -s hello /root/hello/hello.ko
>>>>>>>> ffffffffc05e0000 <<----------module_init with value
MODULE NAME BASE
SIZE OBJECT FILE
ffffffffc05dd000 hello ffffffffc05db000
12431 /root/hello/hello.ko
What do you think of the solution?
Thanks,
Tao Liu
> and etc. Could you take a look?
>
> Thanks,
> Kazu
--
Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
