[Crash-utility] [Patch v4] Show module taint flags

[Dave Anderson]

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 at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
-------------- next part --------------
A non-text attachment was scrubbed...
Name: module_taints.patch
Type: text/x-patch
Size: 9630 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20130521/43db6eb2/attachment.bin>