Hi,  Sun Feng
Thank you for the patch.

On Mon, Oct 28, 2024 at 11:32 AM <devel-request@lists.crash-utility.osci.io> wrote:
Date: Wed, 23 Oct 2024 08:53:58 +0800
From: Sun Feng <loyou85@gmail.com>
Subject: [Crash-utility] [PATCH] mod: introduce -v option to display
        modules with valid version
To: devel@lists.crash-utility.osci.io
Cc: Sun Feng <loyou85@gmail.com>
Message-ID: <20241023005358.11328-1-loyou85@gmail.com>

With this option, we can get module version easily in kdump,
it's helpful when developing external modules.

It seems to be a specific case?
 

crash> mod -v
NAME   VERSION
ahci   3.0
vxlan  0.1.2.1
dca    1.12.1
...

Signed-off-by: Sun Feng <loyou85@gmail.com>
---
 defs.h    |  3 +++
 help.c    | 12 +++++++++++-
 kernel.c  | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 symbols.c | 44 +++++++++++++++++++++++++++++++++++++++-----
 4 files changed, 98 insertions(+), 7 deletions(-)

diff --git a/defs.h b/defs.h
index e2a9278..f14fcdf 100644
--- a/defs.h
+++ b/defs.h
@@ -2244,6 +2244,7 @@ struct offset_table {                    /* stash of commonly-used offsets */
        long rb_list_head;
        long file_f_inode;
        long page_page_type;
+       long module_version;
 };

 struct size_table {         /* stash of commonly-used sizes */
@@ -2935,6 +2936,7 @@ struct symbol_table_data {

 #define MAX_MOD_NAMELIST (256)
 #define MAX_MOD_NAME     (64)
+#define MAX_MOD_VERSION  (64)
 #define MAX_MOD_SEC_NAME (64)

 #define MOD_EXT_SYMS    (0x1)
@@ -2984,6 +2986,7 @@ struct load_module {
         long mod_size;
         char mod_namelist[MAX_MOD_NAMELIST];
         char mod_name[MAX_MOD_NAME];
+        char mod_version[MAX_MOD_VERSION];
         ulong mod_flags;
        struct syment *mod_symtable;
        struct syment *mod_symend;
diff --git a/help.c b/help.c
index e95ac1d..1bac5e1 100644
--- a/help.c
+++ b/help.c
@@ -5719,7 +5719,7 @@ NULL
 char *help_mod[] = {
 "mod",
 "module information and loading of symbols and debugging data",
-"-s module [objfile] | -d module | -S [directory] [-D|-t|-r|-R|-o|-g]",
+"-s module [objfile] | -d module | -S [directory] [-D|-t|-r|-R|-o|-g|-v]",
 "  With no arguments, this command displays basic information of the currently",
 "  installed modules, consisting of the module address, name, base address,",
 "  size, the object file name (if known), and whether the module was compiled",
@@ -5791,6 +5791,7 @@ char *help_mod[] = {
 "                   -g  When used with -s or -S, add a module object's section",
 "                       start and end addresses to its symbol list.",
 "                   -o  Load module symbols with old mechanism.",
+"                   -v  Display modules with valid version.",
 " ",
 "  If the %s session was invoked with the \"--mod <directory>\" option, or",
 "  a CRASH_MODULE_PATH environment variable exists, then /lib/modules/<release>",
@@ -5881,6 +5882,15 @@ char *help_mod[] = {
 "    vxglm     P(U)",
 "    vxgms     P(U)",
 "    vxodm     P(U)",
+" ",
+"  Display modules with valid version:",
+" ",
+"    %s> mod -v",
+"    NAME   VERSION",
+"    ahci   3.0",
+"    vxlan  0.1.2.1",
+"    dca    1.12.1",
+"    ...",
 NULL               
 };

There are many kernel modules, which do not have the actual value for the field "version"(null), E.g: 
crash> struct module c008000005cb1d00
struct module {
...
  version = 0x0,
  srcversion = 0xc00000009c3628c0 "7D7FAEDDA764AC772D6F805",
...

Currently, it is also easy to view the version string, for example:
crash> mod
     MODULE       NAME                             TEXT_BASE         SIZE  OBJECT FILE
c008000004400080  libcrc32c                     c008000004260000   196608  (not loaded)  [CONFIG_KALLSYMS]
...
c0080000044a0700  sg                            c008000004480000   262144  (not loaded)  [CONFIG_KALLSYMS]
...
crash> struct module c0080000044a0700|grep -w version
  version = 0xc000000009d67f20 "3.5.36",

Could you please explain the current background? Why is it needed? As you saw, it's not too hard to get a module version string based on crash internal command.

Thanks
Lianbo
  

diff --git a/kernel.c b/kernel.c
index adb19ad..91eef2a 100644
--- a/kernel.c
+++ b/kernel.c
@@ -3593,6 +3593,9 @@ module_init(void)
                MEMBER_OFFSET_INIT(module_num_gpl_syms, "module",
                        "num_gpl_syms");

+               if (MEMBER_EXISTS("module", "version"))
+                       MEMBER_OFFSET_INIT(module_version, "module", "version");
+
                if (MEMBER_EXISTS("module", "mem")) {   /* 6.4 and later */
                        kt->flags2 |= KMOD_MEMORY;      /* MODULE_MEMORY() can be used. */

@@ -4043,6 +4046,7 @@ irregularity:
 #define REMOTE_MODULE_SAVE_MSG        (6)
 #define REINIT_MODULES                (7)
 #define LIST_ALL_MODULE_TAINT         (8)
+#define LIST_ALL_MODULE_VERSION       (9)

 void
 cmd_mod(void)
@@ -4117,7 +4121,7 @@ cmd_mod(void)
        address = 0;
        flag = LIST_MODULE_HDR;

-        while ((c = getopt(argcnt, args, "Rd:Ds:Sot")) != EOF) {
+        while ((c = getopt(argcnt, args, "Rd:Ds:Sotv")) != EOF) {
                 switch(c)
                {
                 case 'R':
@@ -4195,6 +4199,13 @@ cmd_mod(void)
                                flag = LIST_ALL_MODULE_TAINT;
                        break;

+               case 'v':
+                       if (flag)
+                               cmd_usage(pc->curcmd, SYNOPSIS);
+                       else
+                               flag = LIST_ALL_MODULE_VERSION;
+                       break;
+
                default:
                        argerrs++;
                        break;
@@ -4578,10 +4589,12 @@ do_module_cmd(ulong flag, char *modref, ulong address,
        struct load_module *lm, *lmp;
        int maxnamelen;
        int maxsizelen;
+       int maxversionlen;
        char buf1[BUFSIZE];
        char buf2[BUFSIZE];
        char buf3[BUFSIZE];
        char buf4[BUFSIZE];
+       char buf5[BUFSIZE];

        if (NO_MODULES())
                return;
@@ -4744,6 +4757,37 @@ do_module_cmd(ulong flag, char *modref, ulong address,
        case LIST_ALL_MODULE_TAINT:
                show_module_taint();
                break;
+
+       case LIST_ALL_MODULE_VERSION:
+               maxnamelen = maxversionlen = 0;
+
+               for (i = 0; i < kt->mods_installed; i++) {
+                       lm = &st->load_modules[i];
+                       maxnamelen = strlen(lm->mod_name) > maxnamelen ?
+                               strlen(lm->mod_name) : maxnamelen;
+
+                       maxversionlen = strlen(lm->mod_version) > maxversionlen ?
+                               strlen(lm->mod_version) : maxversionlen;
+               }
+
+               fprintf(fp, "%s  %s\n",
+                       mkstring(buf2, maxnamelen, LJUST, "NAME"),
+                       mkstring(buf5, maxversionlen, LJUST, "VERSION"));
+
+               for (i = 0; i < kt->mods_installed; i++) {
+                       lm = &st->load_modules[i];
+                       if ((!address || (lm->module_struct == address) ||
+                           (lm->mod_base == address)) &&
+                           strlen(lm->mod_version)) {
+                               fprintf(fp, "%s  ", mkstring(buf2, maxnamelen,
+                                       LJUST, lm->mod_name));
+                               fprintf(fp, "%s  ", mkstring(buf5, maxversionlen,
+                                       LJUST, lm->mod_version));
+
+                               fprintf(fp, "\n");
+                       }
+               }
+               break;
        }
 }

diff --git a/symbols.c b/symbols.c
index d00fbd7..9d90df7 100644
--- a/symbols.c
+++ b/symbols.c
@@ -1918,6 +1918,7 @@ store_module_symbols_6_4(ulong total, int mods_installed)
 {
        int i, m, t;
        ulong mod, mod_next;
+       ulong version;
        char *mod_name;
        uint nsyms, ngplsyms;
        ulong syms, gpl_syms;
@@ -1930,6 +1931,7 @@ store_module_symbols_6_4(ulong total, int mods_installed)
        struct load_module *lm;
        char buf1[BUFSIZE];
        char buf2[BUFSIZE];
+       char mod_version[BUFSIZE];
        char *strbuf = NULL, *modbuf, *modsymbuf;
        struct syment *sp;
        ulong first, last;
@@ -1980,6 +1982,13 @@ store_module_symbols_6_4(ulong total, int mods_installed)

                mod_name = modbuf + OFFSET(module_name);

+               BZERO(mod_version, BUFSIZE);
+               if (MEMBER_EXISTS("module", "version")) {
+                       version = ULONG(modbuf + OFFSET(module_version));
+                       if (version)
+                               read_string(version, mod_version, BUFSIZE - 1);
+               }
+
                lm = &st->load_modules[m++];
                BZERO(lm, sizeof(struct load_module));

@@ -2003,9 +2012,15 @@ store_module_symbols_6_4(ulong total, int mods_installed)
                        error(INFO, "module name greater than MAX_MOD_NAME: %s\n", mod_name);
                        strncpy(lm->mod_name, mod_name, MAX_MOD_NAME-1);
                }
+               if (strlen(mod_version) < MAX_MOD_VERSION)
+                       strcpy(lm->mod_version, mod_version);
+               else {
+                       error(INFO, "module version greater than MAX_MOD_VERSION: %s\n", mod_version);
+                       strncpy(lm->mod_version, mod_version, MAX_MOD_VERSION-1);
+               }
                if (CRASHDEBUG(3))
-                       fprintf(fp, "%lx (%lx): %s syms: %d gplsyms: %d ksyms: %ld\n",
-                               mod, lm->mod_base, lm->mod_name, nsyms, ngplsyms, nksyms);
+                       fprintf(fp, "%lx (%lx): %s syms: %d gplsyms: %d ksyms: %ld version: %s\n",
+                               mod, lm->mod_base, lm->mod_name, nsyms, ngplsyms, nksyms, lm->mod_version);

                lm->mod_flags = MOD_EXT_SYMS;
                lm->mod_ext_symcnt = mcnt;
@@ -2271,6 +2286,7 @@ store_module_symbols_v2(ulong total, int mods_installed)
 {
         int i, m;
         ulong mod, mod_next;
+        ulong version;
        char *mod_name;
         uint nsyms, ngplsyms;
         ulong syms, gpl_syms;
@@ -2285,6 +2301,7 @@ store_module_symbols_v2(ulong total, int mods_installed)
        char buf2[BUFSIZE];
        char buf3[BUFSIZE];
        char buf4[BUFSIZE];
+       char mod_version[BUFSIZE];
        char *strbuf, *modbuf, *modsymbuf;
        struct syment *sp;
        ulong first, last;
@@ -2344,6 +2361,13 @@ store_module_symbols_v2(ulong total, int mods_installed)

                mod_name = modbuf + OFFSET(module_name);

+               BZERO(mod_version, BUFSIZE);
+               if (MEMBER_EXISTS("module", "version")) {
+                       version = ULONG(modbuf + OFFSET(module_version));
+                       if (version)
+                               read_string(version, mod_version, BUFSIZE - 1);
+               }
+
                lm = &st->load_modules[m++];
                BZERO(lm, sizeof(struct load_module));
                lm->mod_base = ULONG(modbuf + MODULE_OFFSET2(module_module_core, rx));
@@ -2357,11 +2381,19 @@ store_module_symbols_v2(ulong total, int mods_installed)
                                mod_name);
                        strncpy(lm->mod_name, mod_name, MAX_MOD_NAME-1);
                }
+               if (strlen(mod_version) < MAX_MOD_VERSION)
+                       strcpy(lm->mod_version, mod_version);
+               else {
+                       error(INFO,
+                           "module version greater than MAX_MOD_VERSION: %s\n",
+                               mod_version);
+                       strncpy(lm->mod_version, mod_version, MAX_MOD_VERSION-1);
+               }
                if (CRASHDEBUG(3))
                        fprintf(fp,
-                           "%lx (%lx): %s syms: %d gplsyms: %d ksyms: %ld\n",
-                               mod, lm->mod_base, lm->mod_name, nsyms,
-                               ngplsyms, nksyms);
+                           "%lx (%lx): %s syms: %d gplsyms: %d ksyms: %ld version: %s\n",
+                               mod, lm->mod_base, lm->mod_name, nsyms,
+                               ngplsyms, nksyms, lm->mod_version);
                lm->mod_flags = MOD_EXT_SYMS;
                lm->mod_ext_symcnt = mcnt;
                lm->mod_init_module_ptr = ULONG(modbuf +
@@ -10177,6 +10209,8 @@ dump_offset_table(char *spec, ulong makestruct)
                OFFSET(module_next));
        fprintf(fp, "                   module_name: %ld\n",
                OFFSET(module_name));
+       fprintf(fp, "                module_version: %ld\n",
+               OFFSET(module_version));
        fprintf(fp, "                   module_syms: %ld\n",
                OFFSET(module_syms));
        fprintf(fp, "                  module_nsyms: %ld\n",
--
2.43.0