Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1481340 > unrolled thread

[PATCH v2] taint/module: Clean up global and module taint flags handling

Started byPetr Mladek <pmladek@suse.com>
First post2016-09-12 16:20 +0200
Last post2016-09-14 17:50 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v2] taint/module: Clean up global and module taint flags handling Petr Mladek <pmladek@suse.com> - 2016-09-12 16:20 +0200
    Re: taint/module: Clean up global and module taint flags handling Jessica Yu <jeyu@redhat.com> - 2016-09-13 22:40 +0200
      Re: taint/module: Clean up global and module taint flags handling Petr Mladek <pmladek@suse.com> - 2016-09-14 17:50 +0200

#1481340 — [PATCH v2] taint/module: Clean up global and module taint flags handling

FromPetr Mladek <pmladek@suse.com>
Date2016-09-12 16:20 +0200
Subject[PATCH v2] taint/module: Clean up global and module taint flags handling
Message-ID<sgAfw-4o0-17@gated-at.bofh.it>
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 */
+	bool module;	/* also show as a per-module taint flag */
+};
+
+extern const struct taint_flag taint_flags[TAINT_FLAGS_COUNT];
 
 extern const char hex_asc[];
 #define hex_asc_lo(x)	hex_asc[((x) & 0x0f)]
diff --git a/include/linux/module.h b/include/linux/module.h
index 0c3207d26ac0..f6ee569c62bb 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -399,7 +399,7 @@ struct module {
 	/* Arch-specific module values */
 	struct mod_arch_specific arch;
 
-	unsigned int taints;	/* same bits as kernel:tainted */
+	unsigned long taints;	/* same bits as kernel:taint_flags */
 
 #ifdef CONFIG_GENERIC_BUG
 	/* Support for BUG */
diff --git a/kernel/module.c b/kernel/module.c
index 529efae9f481..303e03e08b51 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -330,7 +330,7 @@ static inline void add_taint_module(struct module *mod, unsigned flag,
 				    enum lockdep_ok lockdep_ok)
 {
 	add_taint(flag, lockdep_ok);
-	mod->taints |= (1U << flag);
+	set_bit(flag, &mod->taints);
 }
 
 /*
@@ -1138,22 +1138,13 @@ static inline int module_unload_init(struct module *mod)
 static size_t module_flags_taint(struct module *mod, char *buf)
 {
 	size_t l = 0;
+	int i;
+
+	for (i = 0; i < TAINT_FLAGS_COUNT; i++) {
+		if (taint_flags[i].module && test_bit(i, &mod->taints))
+			buf[l++] = taint_flags[i].true;
+	}
 
-	if (mod->taints & (1 << TAINT_PROPRIETARY_MODULE))
-		buf[l++] = 'P';
-	if (mod->taints & (1 << TAINT_OOT_MODULE))
-		buf[l++] = 'O';
-	if (mod->taints & (1 << TAINT_FORCED_MODULE))
-		buf[l++] = 'F';
-	if (mod->taints & (1 << TAINT_CRAP))
-		buf[l++] = 'C';
-	if (mod->taints & (1 << TAINT_UNSIGNED_MODULE))
-		buf[l++] = 'E';
-	/*
-	 * TAINT_FORCED_RMMOD: could be added.
-	 * TAINT_CPU_OUT_OF_SPEC, TAINT_MACHINE_CHECK, TAINT_BAD_PAGE don't
-	 * apply to modules.
-	 */
 	return l;
 }
 
@@ -4036,6 +4027,10 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
 }
 #endif /* CONFIG_KALLSYMS */
 
+/* Maximum number of characters written by module_flags() */
+#define MODULE_FLAGS_BUF_SIZE (TAINT_FLAGS_COUNT + 4)
+
+/* Keep in sync with MODULE_FLAGS_BUF_SIZE !!! */
 static char *module_flags(struct module *mod, char *buf)
 {
 	int bx = 0;
@@ -4080,7 +4075,7 @@ static void m_stop(struct seq_file *m, void *p)
 static int m_show(struct seq_file *m, void *p)
 {
 	struct module *mod = list_entry(p, struct module, list);
-	char buf[8];
+	char buf[MODULE_FLAGS_BUF_SIZE];
 
 	/* We always ignore unformed modules. */
 	if (mod->state == MODULE_STATE_UNFORMED)
@@ -4251,7 +4246,7 @@ EXPORT_SYMBOL_GPL(__module_text_address);
 void print_modules(void)
 {
 	struct module *mod;
-	char buf[8];
+	char buf[MODULE_FLAGS_BUF_SIZE];
 
 	printk(KERN_DEFAULT "Modules linked in:");
 	/* Most callers should already have preempt disabled, but make sure */
diff --git a/kernel/panic.c b/kernel/panic.c
index ca8cea1ef673..36d4fa264b2c 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -265,30 +265,27 @@ void panic(const char *fmt, ...)
 
 EXPORT_SYMBOL(panic);
 
-
-struct tnt {
-	u8	bit;
-	char	true;
-	char	false;
-};
-
-static const struct tnt tnts[] = {
-	{ TAINT_PROPRIETARY_MODULE,	'P', 'G' },
-	{ TAINT_FORCED_MODULE,		'F', ' ' },
-	{ TAINT_CPU_OUT_OF_SPEC,	'S', ' ' },
-	{ TAINT_FORCED_RMMOD,		'R', ' ' },
-	{ TAINT_MACHINE_CHECK,		'M', ' ' },
-	{ TAINT_BAD_PAGE,		'B', ' ' },
-	{ TAINT_USER,			'U', ' ' },
-	{ TAINT_DIE,			'D', ' ' },
-	{ TAINT_OVERRIDDEN_ACPI_TABLE,	'A', ' ' },
-	{ TAINT_WARN,			'W', ' ' },
-	{ TAINT_CRAP,			'C', ' ' },
-	{ TAINT_FIRMWARE_WORKAROUND,	'I', ' ' },
-	{ TAINT_OOT_MODULE,		'O', ' ' },
-	{ TAINT_UNSIGNED_MODULE,	'E', ' ' },
-	{ TAINT_SOFTLOCKUP,		'L', ' ' },
-	{ TAINT_LIVEPATCH,		'K', ' ' },
+/*
+ * 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 */
 };
 
 /**
@@ -315,16 +312,16 @@ static const struct tnt tnts[] = {
  */
 const char *print_tainted(void)
 {
-	static char buf[ARRAY_SIZE(tnts) + sizeof("Tainted: ")];
+	static char buf[TAINT_FLAGS_COUNT + sizeof("Tainted: ")];
 
 	if (tainted_mask) {
 		char *s;
 		int i;
 
 		s = buf + sprintf(buf, "Tainted: ");
-		for (i = 0; i < ARRAY_SIZE(tnts); i++) {
-			const struct tnt *t = &tnts[i];
-			*s++ = test_bit(t->bit, &tainted_mask) ?
+		for (i = 0; i < TAINT_FLAGS_COUNT; i++) {
+			const struct taint_flag *t = &taint_flags[i];
+			*s++ = test_bit(i, &tainted_mask) ?
 					t->true : t->false;
 		}
 		*s = 0;
-- 
1.8.5.6

[toc] | [next] | [standalone]


#1482760 — Re: taint/module: Clean up global and module taint flags handling

FromJessica Yu <jeyu@redhat.com>
Date2016-09-13 22:40 +0200
SubjectRe: taint/module: Clean up global and module taint flags handling
Message-ID<sh2EN-6PW-5@gated-at.bofh.it>
In reply to#1481340
+++ 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

>+	bool module;	/* also show as a per-module taint flag */
>+};
>+
>+extern const struct taint_flag taint_flags[TAINT_FLAGS_COUNT];
>
> extern const char hex_asc[];
> #define hex_asc_lo(x)	hex_asc[((x) & 0x0f)]
>diff --git a/include/linux/module.h b/include/linux/module.h
>index 0c3207d26ac0..f6ee569c62bb 100644
>--- a/include/linux/module.h
>+++ b/include/linux/module.h
>@@ -399,7 +399,7 @@ struct module {
> 	/* Arch-specific module values */
> 	struct mod_arch_specific arch;
>
>-	unsigned int taints;	/* same bits as kernel:tainted */
>+	unsigned long taints;	/* same bits as kernel:taint_flags */
>
> #ifdef CONFIG_GENERIC_BUG
> 	/* Support for BUG */
>diff --git a/kernel/module.c b/kernel/module.c
>index 529efae9f481..303e03e08b51 100644
>--- a/kernel/module.c
>+++ b/kernel/module.c
>@@ -330,7 +330,7 @@ static inline void add_taint_module(struct module *mod, unsigned flag,
> 				    enum lockdep_ok lockdep_ok)
> {
> 	add_taint(flag, lockdep_ok);
>-	mod->taints |= (1U << flag);
>+	set_bit(flag, &mod->taints);
> }
>
> /*
>@@ -1138,22 +1138,13 @@ static inline int module_unload_init(struct module *mod)
> static size_t module_flags_taint(struct module *mod, char *buf)
> {
> 	size_t l = 0;
>+	int i;
>+
>+	for (i = 0; i < TAINT_FLAGS_COUNT; i++) {
>+		if (taint_flags[i].module && test_bit(i, &mod->taints))
>+			buf[l++] = taint_flags[i].true;
>+	}
>
>-	if (mod->taints & (1 << TAINT_PROPRIETARY_MODULE))
>-		buf[l++] = 'P';
>-	if (mod->taints & (1 << TAINT_OOT_MODULE))
>-		buf[l++] = 'O';
>-	if (mod->taints & (1 << TAINT_FORCED_MODULE))
>-		buf[l++] = 'F';
>-	if (mod->taints & (1 << TAINT_CRAP))
>-		buf[l++] = 'C';
>-	if (mod->taints & (1 << TAINT_UNSIGNED_MODULE))
>-		buf[l++] = 'E';
>-	/*
>-	 * TAINT_FORCED_RMMOD: could be added.
>-	 * TAINT_CPU_OUT_OF_SPEC, TAINT_MACHINE_CHECK, TAINT_BAD_PAGE don't
>-	 * apply to modules.
>-	 */
> 	return l;
> }
>
>@@ -4036,6 +4027,10 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
> }
> #endif /* CONFIG_KALLSYMS */
>
>+/* Maximum number of characters written by module_flags() */
>+#define MODULE_FLAGS_BUF_SIZE (TAINT_FLAGS_COUNT + 4)
>+
>+/* Keep in sync with MODULE_FLAGS_BUF_SIZE !!! */
> static char *module_flags(struct module *mod, char *buf)
> {
> 	int bx = 0;
>@@ -4080,7 +4075,7 @@ static void m_stop(struct seq_file *m, void *p)
> static int m_show(struct seq_file *m, void *p)
> {
> 	struct module *mod = list_entry(p, struct module, list);
>-	char buf[8];
>+	char buf[MODULE_FLAGS_BUF_SIZE];
>
> 	/* We always ignore unformed modules. */
> 	if (mod->state == MODULE_STATE_UNFORMED)
>@@ -4251,7 +4246,7 @@ EXPORT_SYMBOL_GPL(__module_text_address);
> void print_modules(void)
> {
> 	struct module *mod;
>-	char buf[8];
>+	char buf[MODULE_FLAGS_BUF_SIZE];
>
> 	printk(KERN_DEFAULT "Modules linked in:");
> 	/* Most callers should already have preempt disabled, but make sure */
>diff --git a/kernel/panic.c b/kernel/panic.c
>index ca8cea1ef673..36d4fa264b2c 100644
>--- a/kernel/panic.c
>+++ b/kernel/panic.c
>@@ -265,30 +265,27 @@ void panic(const char *fmt, ...)
>
> EXPORT_SYMBOL(panic);
>
>-
>-struct tnt {
>-	u8	bit;
>-	char	true;
>-	char	false;
>-};
>-
>-static const struct tnt tnts[] = {
>-	{ TAINT_PROPRIETARY_MODULE,	'P', 'G' },
>-	{ TAINT_FORCED_MODULE,		'F', ' ' },
>-	{ TAINT_CPU_OUT_OF_SPEC,	'S', ' ' },
>-	{ TAINT_FORCED_RMMOD,		'R', ' ' },
>-	{ TAINT_MACHINE_CHECK,		'M', ' ' },
>-	{ TAINT_BAD_PAGE,		'B', ' ' },
>-	{ TAINT_USER,			'U', ' ' },
>-	{ TAINT_DIE,			'D', ' ' },
>-	{ TAINT_OVERRIDDEN_ACPI_TABLE,	'A', ' ' },
>-	{ TAINT_WARN,			'W', ' ' },
>-	{ TAINT_CRAP,			'C', ' ' },
>-	{ TAINT_FIRMWARE_WORKAROUND,	'I', ' ' },
>-	{ TAINT_OOT_MODULE,		'O', ' ' },
>-	{ TAINT_UNSIGNED_MODULE,	'E', ' ' },
>-	{ TAINT_SOFTLOCKUP,		'L', ' ' },
>-	{ TAINT_LIVEPATCH,		'K', ' ' },
>+/*
>+ * 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 think the rest looks fine, thanks for working on the cleanups.

Jessica

> };
>
> /**
>@@ -315,16 +312,16 @@ static const struct tnt tnts[] = {
>  */
> const char *print_tainted(void)
> {
>-	static char buf[ARRAY_SIZE(tnts) + sizeof("Tainted: ")];
>+	static char buf[TAINT_FLAGS_COUNT + sizeof("Tainted: ")];
>
> 	if (tainted_mask) {
> 		char *s;
> 		int i;
>
> 		s = buf + sprintf(buf, "Tainted: ");
>-		for (i = 0; i < ARRAY_SIZE(tnts); i++) {
>-			const struct tnt *t = &tnts[i];
>-			*s++ = test_bit(t->bit, &tainted_mask) ?
>+		for (i = 0; i < TAINT_FLAGS_COUNT; i++) {
>+			const struct taint_flag *t = &taint_flags[i];
>+			*s++ = test_bit(i, &tainted_mask) ?
> 					t->true : t->false;
> 		}
> 		*s = 0;
>-- 
>1.8.5.6
>

[toc] | [prev] | [next] | [standalone]


#1483446 — Re: taint/module: Clean up global and module taint flags handling

FromPetr Mladek <pmladek@suse.com>
Date2016-09-14 17:50 +0200
SubjectRe: taint/module: Clean up global and module taint flags handling
Message-ID<shkBI-2Dg-33@gated-at.bofh.it>
In reply to#1482760
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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web