On 2023/11/14 17:32, Tao Liu wrote:
> There might be address overlap of one module's .init.text symbols and
> another module's .text symbols. As a result, gdb fails to translate the
> address to symbol name correctly:
>
> crash> sym -m virtio_blk | grep MODULE
> ffffffffc00a4000 MODULE START: virtio_blk
> ffffffffc00a86ec MODULE END: virtio_blk
> crash> gdb info address floppy_module_init
> Symbol "floppy_module_init" is a function at address 0xffffffffc00a4131.
>
> Since the .init.* sections of a module had been freed by kernel if the
> module was initialized successfully, there is no need to load the .init.*
> sections data from "*.ko.debug" in gdb to create such an overlap.
> lm->mod_init_module_ptr is used as a flag of whether module is freed.
>
> Without the patch:
> crash> mod -S
> crash> struct blk_mq_ops 0xffffffffc00a7160
> struct blk_mq_ops {
> queue_rq = 0xffffffffc00a45b0 <floppy_module_init+1151>, <-- symbol translated from module floppy
> map_queue = 0xffffffff813015c0 <blk_mq_map_queue>,
> ...snip...
> complete = 0xffffffffc00a4370 <floppy_module_init+575>,
> init_request = 0xffffffffc00a4260 <floppy_module_init+303>,
> ...snip...
> }
>
> With the patch:
> crash> mod -S
> crash> struct blk_mq_ops 0xffffffffc00a7160
> struct blk_mq_ops {
> queue_rq = 0xffffffffc00a45b0 <virtio_queue_rq>, <-- symbol translated from module virtio_blk
> map_queue = 0xffffffff813015c0 <blk_mq_map_queue>,
> ...snip...
> complete = 0xffffffffc00a4370 <virtblk_request_done>,
> init_request = 0xffffffffc00a4260 <virtblk_init_request>,
> ...snip...
> }
>
> Signed-off-by: Tao Liu <ltao@xxxxxxxxxx>
> ---
>
> v1: [PATCH 1/2] symbols: expand kernel modules symtable before symbols translation
> [PATCH 2/2] symbols: fix the error belonging of the kernel modules symbols
> v2 -> v1: Used different solution, re-drafted patch based on Kazu's comments,
> so v1 can be discarded.
Thank you for the update, looks good.
Acked-by: Kazuhito Hagio <k-hagio-ab@xxxxxxx>
We're going to release a new version of crash in a few days, but I would
like to evaluate this kind of patch for a while in actual cases. So
this will be not included in the release, sorry about that.
Thanks,
Kazu
>
> ---
> symbols.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/symbols.c b/symbols.c
> index 8e8b4c3..dae5b04 100644
> --- a/symbols.c
> +++ b/symbols.c
> @@ -13283,7 +13283,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 (lm->mod_init_module_ptr || !STRNEQ(section_name, ".init.")) {
> sprintf(buf, " -s %s 0x%lx", section_name, section_vaddr);
> while ((len + strlen(buf)) >= buflen) {
> RESIZEBUF(req->buf, buflen, buflen * 2);
--
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
