[Crash-utility] Re: [PATCH CRASH] Fix module section load address when sh_addr != 0

Tao Liu <ltao(a)redhat.com&gt; writes: Yes, that's correct. Indeed, I checked with fuse on Fedora (6.11.4-301.fc41.x86_64, so almost the same version). And I see the same behavior with fuse_miscdevice. In your case and mine, the offset is 0x1d0c0, which could be a clue to how this mismatch happens. From what I can tell, the important difference here may be that the symbol you selected is a static variable (which becomes a local ELF symbol). Apparently, my patch only resolves the issue for *global variables*, but not for local variables. $ eu-readelf -s /usr/lib/debug/lib/modules/$(uname -r)/kernel/fs/fuse/fuse.ko.debug | grep -P "fuse_(miscdevice|mutex)" 85: 0000000000000100 80 OBJECT LOCAL DEFAULT 50 fuse_miscdevice 1041: 00000000000005c0 32 OBJECT GLOBAL DEFAULT 50 fuse_mutex Here is the behavior of crash's master branch on these two symbols: crash> sym fuse_miscdevice ffffffffc02fafe0 (?) fuse_miscdevice [fuse] crash> sym fuse_mutex ffffffffc02fb4a0 (?) fuse_mutex [fuse] crash> mod -s fuse MODULE NAME TEXT_BASE SIZE OBJECT FILE ffffffffc02fbdc0 fuse ffffffffc02dd000 233472 /usr/lib/debug/lib/modules/6.11.4-301.fc41.x86_64/kernel/fs/fuse/fuse.ko.debug crash> sym fuse_miscdevice ffffffffc02fafe0 (b) fuse_miscdevice [fuse] crash> sym fuse_mutex ffffffffc02fb4a0 (B) fuse_mutex [fuse] crash> p/x &fuse_miscdevice $1 = 0xffffffffc02ddf20 crash> p/x &fuse_mutex $2 = 0xffffffffc02fa680 Both fuse_miscdevice, and fuse_mutex are mismatched (with different offsets). And now, here's the behavior of crash with my patch: crash> sym fuse_miscdevice ffffffffc02fafe0 (?) fuse_miscdevice [fuse] crash> sym fuse_mutex ffffffffc02fb4a0 (?) fuse_mutex [fuse] crash> mod -s fuse MODULE NAME TEXT_BASE SIZE OBJECT FILE ffffffffc02fbdc0 fuse ffffffffc02dd000 233472 /usr/lib/debug/lib/modules/6.11.4-301.fc41.x86_64/kernel/fs/fuse/fuse.ko.debug crash> sym fuse_miscdevice ffffffffc02fafe0 (b) fuse_miscdevice [fuse] crash> sym fuse_mutex ffffffffc02fb4a0 (B) fuse_mutex [fuse] crash> p/x &fuse_miscdevice $1 = 0xffffffffc02ddf20 crash> p/x &fuse_mutex $2 = 0xffffffffc02fb4a0 Here the global fuse_mutex is correct, but the static/local fuse_miscdevice is still incorrect. This indicates that there's more yet to fix. (But at least my patch does part of the work!) I'm wondering whether this has anything to do with ELF relocations. From what I can tell using "readelf -r" and "llvm-dwarfdump", the relocations for the location of these variables are different. Relocations for global variables seem to be done via its corresponding symbol, e.g: Offset Info Type Sym. Value Sym. Name + Addend 00000008bb6b 041100000001 R_X86_64_64 00000000000005c0 fuse_mutex + 0 Whereas relocations for the local variable seem to be done relative to the ".data" section symbol: Offset Info Type Sym. Value Sym. Name + Addend 0000000133f4 001e00000001 R_X86_64_64 0000000000000000 .data + 100 These offsets correspond roughly to what you'd see for the DWARF offsets of the location expressions for each variable from llvm-dwarfdump. So one theory here could be that GDB is not using the provided ".data" address to perform the relocation correctly. I've been playing with the internals of GDB to try to get more information. It seems like "fuse_mutex" is described as "unresolved" whereas "fuse_miscdevice" is shown as being part of the .text section with the same incorrect address. crash> gdb maint print symbols -objfile fuse.ko.debug | grep fuse_mutex crash> p/x &fuse_mutex $1 = 0xffffffffc02fb4a0 crash> gdb maint print symbols -objfile fuse.ko.debug | grep fuse_mutex } fuse_mutex; unresolved crash> gdb maint print symbols -objfile fuse.ko.debug | grep fuse_miscdevice } fuse_miscdevice; static at 0xffffffffc02ddf20 section .text I'm not 100% certain if this is relevant, at this point I'm just sharing debugging information in case it helps you or anybody else. I'll continue looking into the root cause of the issue for static variables. Thanks, Stephen > Thanks, > Tao Liu > > On Wed, Apr 2, 2025 at 11:41 AM Stephen Brennan > <stephen.s.brennan(a)oracle.com&gt; wrote: >> >> A user reported that crash was reporting the address for certain module >> variables incorrectly. I was able to track it down specifically to >> variables which were located in the .data section of a kernel module. >> While the "sym" command gave the correct value, printing the address of >> the variable or expressions based on it with "p" would give an incorrect >> value. For example, the variable "ata_dummy_port_ops" variable is >> included in the .data section of libata.ko when built as a module: >> >> $ sudo grep '\bata_dummy_port_ops\b' /proc/kallsyms >> ffffffffc0a71580 d ata_dummy_port_ops [libata] >> $ sudo crash /usr/lib/debug/lib/modules/$(uname -r)/vmlinux /proc/kcore >> crash> sym ata_dummy_port_ops >> ffffffffc0a71580 (?) ata_dummy_port_ops [libata] >> crash> mod -s libata >> MODULE NAME TEXT_BASE SIZE OBJECT FILE >> ffffffffc0a7b640 libata ffffffffc0a47000 520192 >> /usr/lib/debug/lib/modules/6.12.0-0.11.8.el9uek.x86_64/kernel/drivers/ata/libata.ko.debug >> crash> sym ata_dummy_port_ops >> ffffffffc0a71580 (B) ata_dummy_port_ops [libata] >> crash> p/x &ata_dummy_port_ops >> $1 = 0xffffffffc0a6fe80 >> >> The symbol value (from kallsyms) is correct, but its address provided by >> GDB is incorrect. It turns out that the .data section has an sh_addr >> which is non-zero. The result of this is that calculate_load_order_6_4() >> incorrectly calculates the base address for the .data section. This >> patch fixes the base address which is later provided to GDB via >> add-symbol-file. >> >> The impact here is interesting. Only variables within sections that have >> a non-zero sh_addr are impacted. It turns out that this is relatively >> common since Linux kernel commit 22d407b164ff7 ("lib: add allocation >> tagging support for memory allocation profiling"), which was merged in >> 6.10. That commit added an entry to the scripts/module.lds.S linker >> script, without specifying a base address of zero. I believe that is the >> reason for the non-zero base addresses. >> >> I was able to verify that, in addition to the Oracle Linux kernel where >> we initially noticed the issue, kernel modules on Arch Linux and Fedora >> also have non-zero .data sh_addr values. This is likely the case for >> most non-clang kernels since 6.10, but those were the only two distros I >> checked. While my reading of the module.lds.S seems to indicate that >> kernels built with CONFIG_LTO_CLANG=y should also have non-zero .data, >> .bss, and .rodata section addresses, I haven't been able to reproduce >> this with clang LTO kernels. Regardless, crash should properly handle >> non-zero sh_addr since it exists in the real world now. >> >> The core of the issue is that the symbol value returned by BFD includes >> the sh_addr of the section containing the symbol. For example, suppose >> a symbol with address 0 is located within a section with virtual address >> 0xa00. Then, the resulting symbol value will be 0xa00, not 0. >> >> calculate_load_order_6_4() computes the base address of each section by >> using a kallsyms symbol known to be within that section, and then >> subtracting the value of the symbol from the object file. This >> implicitly assumes that the section sh_addr is zero, and thus the symbol >> value is just an offset. To fix the computation, add in the section base >> address, to account for cases where it is non-zero. >> >> Signed-off-by: Stephen Brennan <stephen.s.brennan(a)oracle.com&gt; >> --- >> symbols.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/symbols.c b/symbols.c >> index 5adbc30..e30fafe 100644 >> --- a/symbols.c >> +++ b/symbols.c >> @@ -12808,6 +12808,7 @@ calculate_load_order_6_4(struct load_module *lm, bfd *bfd, int dynamic, >> asymbol *store; >> asymbol *sym; >> symbol_info syminfo; >> + bfd_vma secaddr; >> char *secname; >> int i, t; >> >> @@ -12860,6 +12861,7 @@ calculate_load_order_6_4(struct load_module *lm, bfd *bfd, int dynamic, >> } >> if (strcmp(syminfo.name, s1->name) == 0) { >> secname = (char *)bfd_section_name(sym->section); >> + secaddr = bfd_section_vma(sym->section); >> break; >> } >> >> @@ -12890,14 +12892,14 @@ calculate_load_order_6_4(struct load_module *lm, bfd *bfd, int dynamic, >> } >> >> /* Update the offset information for the section */ >> - sec_start = s1->value - syminfo.value; >> + sec_start = s1->value - syminfo.value + secaddr; >> /* keep the address instead of offset */ >> lm->mod_section_data[i].addr = sec_start; >> lm->mod_section_data[i].flags |= SEC_FOUND; >> >> if (CRASHDEBUG(2)) >> - fprintf(fp, "update sec offset sym %s @ %lx val %lx section %s\n", >> - s1->name, s1->value, (ulong)syminfo.value, secname); >> + fprintf(fp, "update sec offset sym %s @ %lx val %lx section %s @ %lx\n", >> + s1->name, s1->value, (ulong)syminfo.value, secname, secaddr); >> >> if (strcmp(secname, ".text") == 0) >> lm->mod_text_start = sec_start; >> -- >> 2.43.5 >> -- >> Crash-utility mailing list -- devel(a)lists.crash-utility.osci.io >> To unsubscribe send an email to devel-leave(a)lists.crash-utility.osci.io >> https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/ >> Contribution Guidelines: https://github.com/crash-utility/crash/wiki