Re: [Patch v4] Show module taint flags

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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

[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux