From: Petr Mladek <pmladek@suse.com>
To: Jessica Yu <jeyu@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>,
Andrew Morton <akpm@linux-foundation.org>,
Jiri Kosina <jkosina@suse.cz>,
Josh Poimboeuf <jpoimboe@redhat.com>,
live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: taint/module: Clean up global and module taint flags handling
Date: Wed, 14 Sep 2016 17:45:37 +0200 [thread overview]
Message-ID: <20160914154537.GB8204@dhcp128.suse.cz> (raw)
In-Reply-To: <20160913203608.GA1863@packer-debian-8-amd64.digitalocean.com>
On Tue 2016-09-13 16:36:09, Jessica Yu wrote:
> +++ Petr Mladek [12/09/16 16:13 +0200]:
> >The commit 66cc69e34e86a231 ("Fix: module signature vs tracepoints:
> >add new TAINT_UNSIGNED_MODULE") updated module_taint_flags() to
> >potentially print one more character. But it did not increase the
> >size of the corresponding buffers in m_show() and print_modules().
> >
> >We have recently done the same mistake when adding a taint flag
> >for livepatching, see
> >https://lkml.kernel.org/g/cfba2c823bb984690b73572aaae1db596b54a082.1472137475.git.jpoimboe@redhat.com
> >
> >Also struct module uses an incompatible type for mod-taints flags.
> >It survived from the commit 2bc2d61a9638dab670d ("[PATCH] list module
> >taint flags in Oops/panic"). There was used "int" for the global taint
> >flags at these times. But only the global tain flags was later changed
> >to "unsigned long" by the commit 25ddbb18aae33ad2 ("Make the taint
> >flags reliable").
> >
> >This patch defines TAINT_FLAGS_COUNT that can be used to create
> >arrays and buffers of the right size. Note that we could not use
> >enum because the taint flag indexes are used also in assembly code.
> >
> >Then it reworks the table that describes the taint flags. The TAINT_*
> >numbers can be used as the index. Instead, we add information
> >if the taint flag is also shown per-module.
> >
> >Finally, it uses "unsigned long", bit operations, and the updated
> >taint_flags table also for mod->taints.
> >
> >It is not optimal because only few taint flags can be printed by
> >module_taint_flags(). But better be on the safe side. IMHO, it is
> >not worth the optimization and this is a good compromise.
> >
> >Signed-off-by: Petr Mladek <pmladek@suse.com>
> >---
> >
> >Changes against v1:
> >
> > + reverted the change to enums because it broke asm code
> >
> > + instead, forced the size of the taint_flags table;
> > used taint numbers as the index; used the table also
> > in module.c
> >
> > + fixed the type of mod->taints
> >
> >
> >include/linux/kernel.h | 9 +++++++++
> >include/linux/module.h | 2 +-
> >kernel/module.c | 31 +++++++++++++----------------
> >kernel/panic.c | 53 ++++++++++++++++++++++++--------------------------
> >4 files changed, 48 insertions(+), 47 deletions(-)
> >
> >diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> >index d96a6118d26a..33e88ff3af40 100644
> >--- a/include/linux/kernel.h
> >+++ b/include/linux/kernel.h
> >@@ -509,6 +509,15 @@ extern enum system_states {
> >#define TAINT_UNSIGNED_MODULE 13
> >#define TAINT_SOFTLOCKUP 14
> >#define TAINT_LIVEPATCH 15
> >+#define TAINT_FLAGS_COUNT 16
> >+
> >+struct taint_flag {
> >+ char true; /* character printed when tained */
> >+ char false; /* character printed when not tained */
>
> s/tained/tainted
Great catch!
> >diff --git a/kernel/panic.c b/kernel/panic.c
> >index ca8cea1ef673..36d4fa264b2c 100644
> >--- a/kernel/panic.c
> >+++ b/kernel/panic.c
> >+/*
> >+ * TAINT_FORCED_RMMOD could be a per-module flag but the module
> >+ * is being removed anyway.
> >+ */
> >+const struct taint_flag taint_flags[TAINT_FLAGS_COUNT] = {
> >+ { 'P', 'G', true }, /* TAINT_PROPRIETARY_MODULE */
> >+ { 'F', ' ', true }, /* TAINT_FORCED_MODULE */
> >+ { 'S', ' ', false }, /* TAINT_CPU_OUT_OF_SPEC */
> >+ { 'R', ' ', false }, /* TAINT_FORCED_RMMOD */
> >+ { 'M', ' ', false }, /* TAINT_MACHINE_CHECK */
> >+ { 'B', ' ', false }, /* TAINT_BAD_PAGE */
> >+ { 'U', ' ', false }, /* TAINT_USER */
> >+ { 'D', ' ', false }, /* TAINT_DIE */
> >+ { 'A', ' ', false }, /* TAINT_OVERRIDDEN_ACPI_TABLE */
> >+ { 'W', ' ', false }, /* TAINT_WARN */
> >+ { 'C', ' ', true }, /* TAINT_CRAP */
> >+ { 'I', ' ', false }, /* TAINT_FIRMWARE_WORKAROUND */
> >+ { 'O', ' ', true }, /* TAINT_OOT_MODULE */
> >+ { 'E', ' ', true }, /* TAINT_UNSIGNED_MODULE */
> >+ { 'L', ' ', false }, /* TAINT_SOFTLOCKUP */
> >+ { 'K', ' ', false }, /* TAINT_LIVEPATCH */
>
> This should be true here, right? TAINT_LIVEPATCH has been made a
> module-specific taint by commit 2992ef29ae ("livepatch/module: make
> TAINT_LIVEPATCH module-specific").
I was not sure whose maintainer tree would be used. So, I rather based
this on the Linus' stable tree. If it will go via the Jikos' livepatch
I could rebase it on top of the commit 2992ef29ae ("livepatch/module: make
TAINT_LIVEPATCH module-specific").
> I think the rest looks fine, thanks for working on the cleanups.
Thanks for review.
Best Regards,
Petr
next prev parent reply other threads:[~2016-09-14 15:45 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-12 14:13 [PATCH v2] taint/module: Clean up global and module taint flags handling Petr Mladek
2016-09-13 20:36 ` Jessica Yu
2016-09-14 15:45 ` Petr Mladek [this message]
2016-09-21 11:47 [PATCH v3] " Petr Mladek
2016-09-23 4:48 ` Jessica Yu
2016-09-23 12:45 ` Jiri Kosina
2016-10-07 7:57 ` Jiri Kosina
2016-10-26 1:07 ` Rusty Russell
2016-10-26 8:17 ` Jiri Kosina
2016-10-26 23:14 ` Jessica Yu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160914154537.GB8204@dhcp128.suse.cz \
--to=pmladek@suse.com \
--cc=akpm@linux-foundation.org \
--cc=jeyu@redhat.com \
--cc=jkosina@suse.cz \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=live-patching@vger.kernel.org \
--cc=rusty@rustcorp.com.au \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).