Hi Aaron, With respect to your v4 version of the patch, I just found a few issues that need addressing. The module.gpgsig_ok "unsigned" check was only made if module.taints exists, and not checked if module.license_gplok exists. Earlier kernels have license_gplok/gpgsig_ok combinations. I'm not sure how you were able to get this sample display from your help page: crash> mod -T NAME TAINT dm_mod G scsi_tgt G serio_raw G dm_log G ata_generic G qla2xxx P(U) dm_region_hash G enclosure G pata_acpi G because the module.taints field would be 0 for all but the qla2xxx module, and your code gates the tnts[] true/false display by "if (taints)"? In my tests of your patch, the 'G' would only be shown if some *other* bit in the bitfield were set. Anway, I don't feel that it's necessary to print the 'G' for GPL'd modules. We could probably just drop the "false" bit check entirely, but perhaps there will be something put there in the future that would be worthy of displaying? But it would only get picked up if some *other* bit in the mask were set, so for now it's pretty much useless code. The module maxnamelen should be restricted to the modules that are actually tainted or unsigned. Another issue -- up until 2.6.19, the module->license_gplok was a boolean, where "1" meant it was GPL. RHEL5 completely turned that around, and made license_gplok a bitmask, whereas upstream 2.6.18 still has it as a boolean. So since we're keying on non-zero values in that field, it confuses the issue because, say on a RHEL4 kernel, it will show *all* GPL modules as having a hexadecimal value of 1. But I can't find an obvious way of determining what's in play for a given kernel, so the "consult the kernel sources" reference in the help page will have to suffice. Anyway, in order to get some traction in getting this patch into place, I've reworked it a bit, and have attached my counter-proposal patch. Here's what I've changed: (1) The '-t' and '-T' difference is too confusing. I've changed it to simply be 't', and the display will be symbolic (the letter) if tnts[] exists, and hexadecimal if not. The unsigned indication will always be shown if gdbsig_ok exists and is 0. (2) If module.taints exists, the header shows "TAINTS". For older kernels, it shows "LICENSE_GPLOK". (3) I moved the module structure's gpgsig_ok, taints and license_gplok members, and the tnt structure's bit, true, false members into the offset_table, and tnt structure size into the size_table. This prevents having to dive into gdb each time MEMBER_EXISTS() and MEMBER_OFFSET are called, and will catch any changes in the structure names in the future. (4) When walking through the modules, readmem() each module's module struct into a buffer and then access the relevant fields with the UINT() and INT() macros, making things a bit less verbose. (5) Never show 'G'. (6) Set the maxnamelen only according to tainted/unsigned modules. (7) The help page has been changed to only have '-t' as an option, as well as indicating that the relevant taints/license_gplok field will only be shown if they are non-zero. The kernel source reference is especially relevant given that old kernels with boolean license_gplok fields will show "1" for all modules. Let me know if you have any problems with, or find any bugs in, the attached patch, and if not, let's queue it for crash-7.0.1. Thanks, Dave ----- Original Message ----- > Hi Dave, > > > Since v4: > > - Updated help_mod[] help page > - User is notified if no tainted modules exists > - Added the '-t' option, to display the hexadecimal value of a module's > "taint" flag > > > Examples: > > crash> mod -T > NOTE: modules have changed on this system -- reinitializing > NAME TAINT > test GFO > > crash> mod -t > NAME TAINT > test 0x1002 > > crash> mod -T > NAME TAINT > vxfs P(U) > vxspec P(U) > dmpaa P(U) > dmpap P(U) > dmpjbod P(U) > fdd P(U) > vxportal P(U) > vxdmp P(U) > vxio P(U) > llt P(U) > gab P(U) > vxfen P(U) > amf P(U) > vxodm P(U) > > crash> mod -t > NAME TAINT > vxfs 0x1 > vxspec 0x1 > dmpaa 0x1 > dmpap 0x1 > dmpjbod 0x1 > fdd 0x1 > vxportal 0x1 > vxdmp 0x1 > vxio 0x1 > llt 0x1 > gab 0x1 > vxfen 0x1 > amf 0x1 > vxodm 0x1 > > > Regards, > Aaron > > ---8<--- > > help.c | 23 +++++++- > kernel.c | 181 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 202 insertions(+), 2 deletions(-) > > > -- > Crash-utility mailing list > Crash-utility@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/crash-utility
--- crash-7.0.0/help.c.orig +++ crash-7.0.0/help.c @@ -4495,7 +4495,7 @@ NULL char *help_mod[] = { "mod", "module information and loading of symbols and debugging data", -"-s module [objfile] | -d module | -S [directory] | -D | -r | -R | -o | -g", +"-s module [objfile] | -d module | -S [directory] [-D|-t|-r|-R|-o|-g]", " With no arguments, this command displays basic information of the currently", " installed modules, consisting of the module address, name, size, the", " object file name (if known), and whether the module was compiled with", @@ -4547,6 +4547,17 @@ char *help_mod[] = { " argument is appended, then the search will be restricted", " to that directory.", " -D Deletes the symbolic and debugging data of all modules.", +" -t Display the contents of the module's \"taints\" bitmask", +" if it is non-zero. When possible, the \"taints\" bits", +" are translated to symbolic letters of the taint type;", +" otherwise the hexadecimal value is shown. In older", +" kernels, the contents of the \"license_gplok\" field is", +" displayed in hexadecimal if it is non-zero; the field", +" may be either a bitmask or a boolean, depending upon the", +" kernel version. The relevant kernel sources should be", +" consulted for the meaning of the letter(s) or hexadecimal", +" bit value(s). For modules that have a \"gpgsig_ok\" field", +" that is zero (unsigned), the notation \"(U)\" is shown.", " -r Passes the -readnow flag to the embedded gdb module,", " which will override the two-stage strategy that it uses", " for reading symbol tables from module object files.", @@ -4673,6 +4684,21 @@ char *help_mod[] = { " c806e000 autofs 9316 (not loaded)", " c8072000 nfsd 151896 (not loaded)", " c80a1000 mdacon 3556 (not loaded)", +" ", +" Display modules that are \"tainted\", where in this case", +" where they are proprietary and unsigned:", +" ", +" %s> mod -t", +" NAME TAINT", +" vxspec P(U)", +" vxportal P(U)", +" fdd P(U)", +" vxfs P(U)", +" vxdmp P(U)", +" vxio P(U)", +" vxglm P(U)", +" vxgms P(U)", +" vxodm P(U)", NULL }; --- crash-7.0.0/defs.h.orig +++ crash-7.0.0/defs.h @@ -1867,6 +1867,12 @@ struct offset_table { long ktime_t_tv64; long ktime_t_sec; long ktime_t_nsec; + long module_taints; + long module_gpgsig_ok; + long module_license_gplok; + long tnt_bit; + long tnt_true; + long tnt_false; }; struct size_table { /* stash of commonly-used sizes */ @@ -2006,6 +2012,7 @@ struct size_table { /* stash of long vmap_area; long hrtimer_clock_base; long hrtimer_base; + long tnt; }; struct array_table { --- crash-7.0.0/kernel.c.orig +++ crash-7.0.0/kernel.c @@ -22,6 +22,7 @@ #include <ctype.h> static void do_module_cmd(ulong, char *, ulong, char *, char *); +static void show_module_taint(void); static char *find_module_objfile(char *, char *, char *); static char *module_objfile_search(char *, char *, char *); static char *get_loadavg(char *); @@ -3221,6 +3222,7 @@ irregularity: #define DELETE_ALL_MODULE_SYMBOLS (5) #define REMOTE_MODULE_SAVE_MSG (6) #define REINIT_MODULES (7) +#define LIST_ALL_MODULE_TAINT (8) void cmd_mod(void) @@ -3295,7 +3297,7 @@ cmd_mod(void) address = 0; flag = LIST_MODULE_HDR; - while ((c = getopt(argcnt, args, "Rd:Ds:So")) != EOF) { + while ((c = getopt(argcnt, args, "Rd:Ds:Sot")) != EOF) { switch(c) { case 'R': @@ -3366,6 +3368,13 @@ cmd_mod(void) cmd_usage(pc->curcmd, SYNOPSIS); break; + case 't': + if (flag) + cmd_usage(pc->curcmd, SYNOPSIS); + else + flag = LIST_ALL_MODULE_TAINT; + break; + default: argerrs++; break; @@ -3486,6 +3495,146 @@ check_specified_module_tree(char *module return retval; } +static void +show_module_taint(void) +{ + int i, j, bx; + struct load_module *lm; + int maxnamelen; + int found; + char buf1[BUFSIZE]; + char buf2[BUFSIZE]; + int gpgsig_ok, license_gplok; + struct syment *sp; + uint *taintsp, taints; + uint8_t tnt_bit; + char tnt_true, tnt_false; + int tnts_exists, tnts_len; + ulong tnts_addr; + char *modbuf; + + if (INVALID_MEMBER(module_taints) && + INVALID_MEMBER(module_license_gplok)) { + MEMBER_OFFSET_INIT(module_taints, "module", "taints"); + MEMBER_OFFSET_INIT(module_license_gplok, + "module", "license_gplok"); + MEMBER_OFFSET_INIT(module_gpgsig_ok, "module", "gpgsig_ok"); + STRUCT_SIZE_INIT(tnt, "tnt"); + MEMBER_OFFSET_INIT(tnt_bit, "tnt", "bit"); + MEMBER_OFFSET_INIT(tnt_true, "tnt", "true"); + MEMBER_OFFSET_INIT(tnt_false, "tnt", "false"); + } + + if (INVALID_MEMBER(module_taints) && + INVALID_MEMBER(module_license_gplok)) + option_not_supported('t'); + + modbuf = GETBUF(SIZE(module)); + + for (i = found = maxnamelen = 0; i < kt->mods_installed; i++) { + lm = &st->load_modules[i]; + + readmem(lm->module_struct, KVADDR, modbuf, SIZE(module), + "module struct", FAULT_ON_ERROR); + + taints = VALID_MEMBER(module_taints) ? + UINT(modbuf + OFFSET(module_taints)) : 0; + license_gplok = VALID_MEMBER(module_license_gplok) ? + INT(modbuf + OFFSET(module_license_gplok)) : 0; + gpgsig_ok = VALID_MEMBER(module_gpgsig_ok) ? + INT(modbuf + OFFSET(module_gpgsig_ok)) : 1; + + if (taints || license_gplok || !gpgsig_ok) { + found++; + maxnamelen = strlen(lm->mod_name) > maxnamelen ? + strlen(lm->mod_name) : maxnamelen; + } + + } + + if (!found) { + fprintf(fp, "no tainted modules\n"); + FREEBUF(modbuf); + return; + } + + if (VALID_STRUCT(tnt) && (sp = symbol_search("tnts"))) { + tnts_exists = TRUE; + tnts_len = get_array_length("tnts", NULL, 0); + tnts_addr = sp->value; + } else { + tnts_exists = FALSE; + tnts_len = 0; + tnts_addr = 0; + } + + fprintf(fp, "%s %s\n", + mkstring(buf2, maxnamelen, LJUST, "NAME"), + VALID_MEMBER(module_taints) ? "TAINTS" : "LICENSE_GPLOK"); + + for (i = 0; i < st->mods_installed; i++) { + + lm = &st->load_modules[i]; + bx = 0; + buf1[0] = '\0'; + + readmem(lm->module_struct, KVADDR, modbuf, SIZE(module), + "module struct", FAULT_ON_ERROR); + + taints = VALID_MEMBER(module_taints) ? + UINT(modbuf + OFFSET(module_taints)) : 0; + license_gplok = VALID_MEMBER(module_license_gplok) ? + INT(modbuf + OFFSET(module_license_gplok)) : 0; + gpgsig_ok = VALID_MEMBER(module_gpgsig_ok) ? + INT(modbuf + OFFSET(module_gpgsig_ok)) : 1; + + if (!taints && !license_gplok && gpgsig_ok) + continue; + + if (tnts_exists && taints) { + taintsp = &taints; + for (j = 0; j < (tnts_len * SIZE(tnt)); j += SIZE(tnt)) { + readmem((tnts_addr + j) + OFFSET(tnt_bit), + KVADDR, &tnt_bit, sizeof(uint8_t), + "tnt bit", FAULT_ON_ERROR); + + if (NUM_IN_BITMAP(taintsp, tnt_bit)) { + readmem((tnts_addr + j) + OFFSET(tnt_true), + KVADDR, &tnt_true, sizeof(char), + "tnt true", FAULT_ON_ERROR); + buf1[bx++] = tnt_true; + } else { + readmem((tnts_addr + j) + OFFSET(tnt_false), + KVADDR, &tnt_false, sizeof(char), + "tnt false", FAULT_ON_ERROR); + if (tnt_false != ' ' && tnt_false != '-' && + tnt_false != 'G') + buf1[bx++] = tnt_false; + } + + } + } + + if (VALID_MEMBER(module_gpgsig_ok) && !gpgsig_ok) { + buf1[bx++] = '('; + buf1[bx++] = 'U'; + buf1[bx++] = ')'; + } + + buf1[bx++] = '\0'; + + if (tnts_exists) + fprintf(fp, "%s %s\n", mkstring(buf2, maxnamelen, + LJUST, lm->mod_name), buf1); + else + fprintf(fp, "%s %x%s\n", mkstring(buf2, maxnamelen, + LJUST, lm->mod_name), + VALID_MEMBER(module_taints) ? + taints : license_gplok, buf1); + } + + FREEBUF(modbuf); +} /* * Do the simple list work for cmd_mod(). @@ -3656,6 +3805,10 @@ do_module_cmd(ulong flag, char *modref, reinit_modules(); do_module_cmd(LIST_MODULE_HDR, NULL, 0, NULL, NULL); break; + + case LIST_ALL_MODULE_TAINT: + show_module_taint(); + break; } } --- crash-7.0.0/symbols.c.orig +++ crash-7.0.0/symbols.c @@ -7839,6 +7839,16 @@ dump_offset_table(char *spec, ulong make fprintf(fp, " kallsyms_section_name_off: %ld\n", OFFSET(kallsyms_section_name_off)); + fprintf(fp, " module_taints: %ld\n", + OFFSET(module_taints)); + fprintf(fp, " module_license_gplok: %ld\n", + OFFSET(module_license_gplok)); + fprintf(fp, " module_gpgsig_ok: %ld\n", + OFFSET(module_gpgsig_ok)); + fprintf(fp, " tnt_bit: %ld\n", OFFSET(tnt_bit)); + fprintf(fp, " tnt_true: %ld\n", OFFSET(tnt_true)); + fprintf(fp, " tnt_false: %ld\n", OFFSET(tnt_false)); + fprintf(fp, " page_next: %ld\n", OFFSET(page_next)); fprintf(fp, " page_prev: %ld\n", OFFSET(page_prev)); fprintf(fp, " page_next_hash: %ld\n", @@ -9226,6 +9236,8 @@ dump_offset_table(char *spec, ulong make SIZE(hrtimer_clock_base)); fprintf(fp, " hrtimer_base: %ld\n", SIZE(hrtimer_base)); + fprintf(fp, " tnt: %ld\n", + SIZE(tnt)); fprintf(fp, "\n array_table:\n"); /*
-- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility