On Wed, Nov 15, 2023 at 4:00 PM <devel-request@lists.crash-utility.osci.io> wrote:
Date: Tue, 14 Nov 2023 16:32:07 +0800
From: Tao Liu <ltao@redhat.com>
Subject: [Crash-utility] [PATCH v2] symbols: skip load .init.*
        sections if module was successfully initialized
To: devel@lists.crash-utility.osci.io
Cc: Tao Liu <ltao@redhat.com>
Message-ID: <20231114083206.11202-1-ltao@redhat.com>
Content-Type: text/plain; charset="US-ASCII"; x-default=true

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.


Good findings, Tao.

The v2 looks good to me. So: Ack.

Thanks
Lianbo

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@redhat.com>
---

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.

---
 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);
--
2.40.1