Hi Stephen,
Thanks for reporting the issue and patch. Please check if I understood
you correctly. The correct output you are expecting in your case is:
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 <<-------- should be 0xffffffffc0a71580,
same as sym's output, right?
If that is the case, then after applied your patch, the issue still
exists on my machine:
crash> sym fuse_miscdevice
ffffffffc05f7fe0 (?) fuse_miscdevice [fuse]
crash> mod -s fuse
MODULE NAME TEXT_BASE
SIZE OBJECT FILE
ffffffffc05f8dc0 fuse ffffffffc05da000
233472
/usr/lib/debug/lib/modules/6.11.5-300.fc41.x86_64/kernel/fs/fuse/fuse.ko.debug
crash> sym fuse_miscdevice
ffffffffc05f7fe0 (b) fuse_miscdevice [fuse]
crash> p/x &fuse_miscdevice
$1 = 0xffffffffc05daf20 << ------ unmatch with the previous value.
The .data section of fuse.ko.debug have non-zero address:
$ readelf -S
/usr/lib/debug/lib/modules/6.11.5-300.fc41.x86_64/kernel/fs/fuse/fuse.ko.debug
Section Headers:
[Nr] Name Type Address Offset
Size EntSize Flags Link Info Align
...
[50] .data NOBITS 0000000000000e20 00000100
000000000000080f 0000000000000000 WA 0 0 32
Could you please re-check your patch for this, or is there something
I'm missing?
Thanks,
Tao Liu
On Wed, Apr 2, 2025 at 11:41 AM Stephen Brennan
<stephen.s.bren...@oracle.com> 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.bren...@oracle.com>
> ---
> 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@lists.crash-utility.osci.io
> To unsubscribe send an email to devel-le...@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
--
Crash-utility mailing list -- devel@lists.crash-utility.osci.io
To unsubscribe send an email to devel-le...@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
