Hi Kazu,
On Wed, Nov 15, 2023 at 3:59 PM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab@xxxxxxx> wrote:
>
> 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.
>
OK, no problem, thanks for the info.
Thanks,
Tao Liu
> 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
