2024-03-25 17:06:59

by Jani Nikula

[permalink] [raw]
Subject: [RFC 0/4] kernel/panic: add more descriptive logging of kernel taints

A few years back there was a discussion about it being difficult to
remember all the taint flags [1].

Time flies. I stumbled on my old branch, brushed it up, and here it is.

I'm not entirely happy with the static buf (which was there to begin
with) or how to decide on its size. Thoughts?

BR,
Jani.

[1] https://lore.kernel.org/r/YmvU+/[email protected]


Jani Nikula (4):
kernel/panic: return early from print_tainted() when not tainted
kernel/panic: convert print_tainted() to use struct seq_buf internally
kernel/panic: initialize taint_flags[] using a macro
kernel/panic: add verbose logging of kernel taints in backtraces

include/linux/panic.h | 8 +--
kernel/panic.c | 116 ++++++++++++++++++++++++++++--------------
lib/dump_stack.c | 3 ++
3 files changed, 87 insertions(+), 40 deletions(-)

--
2.39.2


2024-03-25 17:07:07

by Jani Nikula

[permalink] [raw]
Subject: [RFC 1/4] kernel/panic: return early from print_tainted() when not tainted

Reduce indent to make follow-up changes slightly easier on the eyes.

Cc: Andrew Morton <[email protected]>
Cc: Greg KH <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
---
kernel/panic.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/kernel/panic.c b/kernel/panic.c
index 628673ddf1c9..b6d4d1da1eaa 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -509,22 +509,23 @@ const struct taint_flag taint_flags[TAINT_FLAGS_COUNT] = {
const char *print_tainted(void)
{
static char buf[TAINT_FLAGS_COUNT + sizeof("Tainted: ")];
+ char *s;
+ int i;

BUILD_BUG_ON(ARRAY_SIZE(taint_flags) != TAINT_FLAGS_COUNT);

- if (tainted_mask) {
- char *s;
- int i;
-
- s = buf + sprintf(buf, "Tainted: ");
- for (i = 0; i < TAINT_FLAGS_COUNT; i++) {
- const struct taint_flag *t = &taint_flags[i];
- *s++ = test_bit(i, &tainted_mask) ?
- t->c_true : t->c_false;
- }
- *s = 0;
- } else
+ if (!tainted_mask) {
snprintf(buf, sizeof(buf), "Not tainted");
+ return buf;
+ }
+
+ s = buf + sprintf(buf, "Tainted: ");
+ for (i = 0; i < TAINT_FLAGS_COUNT; i++) {
+ const struct taint_flag *t = &taint_flags[i];
+ *s++ = test_bit(i, &tainted_mask) ?
+ t->c_true : t->c_false;
+ }
+ *s = 0;

return buf;
}
--
2.39.2


2024-03-25 17:18:15

by Jani Nikula

[permalink] [raw]
Subject: [RFC 4/4] kernel/panic: add verbose logging of kernel taints in backtraces

With nearly 20 taint flags and respective characters, it's getting a bit
difficult to remember what each taint flag character means. Add verbose
logging of the set taints in the format:

Tainted: [P]=PROPRIETARY_MODULE, [W]=WARN

in dump_stack_print_info() when there are taints.

Note that the "negative flag" G is not included.

Cc: Andrew Morton <[email protected]>
Cc: Greg KH <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
---
include/linux/panic.h | 8 +++++---
kernel/panic.c | 45 ++++++++++++++++++++++++++++++++-----------
lib/dump_stack.c | 3 +++
3 files changed, 42 insertions(+), 14 deletions(-)

diff --git a/include/linux/panic.h b/include/linux/panic.h
index 6717b15e798c..3130e0b5116b 100644
--- a/include/linux/panic.h
+++ b/include/linux/panic.h
@@ -77,9 +77,10 @@ static inline void set_arch_panic_timeout(int timeout, int arch_default_timeout)
#define TAINT_FLAGS_MAX ((1UL << TAINT_FLAGS_COUNT) - 1)

struct taint_flag {
- char c_true; /* character printed when tainted */
- char c_false; /* character printed when not tainted */
- bool module; /* also show as a per-module taint flag */
+ char c_true; /* character printed when tainted */
+ char c_false; /* character printed when not tainted */
+ bool module; /* also show as a per-module taint flag */
+ const char *desc; /* verbose description of the set taint flag */
};

extern const struct taint_flag taint_flags[TAINT_FLAGS_COUNT];
@@ -90,6 +91,7 @@ enum lockdep_ok {
};

extern const char *print_tainted(void);
+extern const char *print_tainted_verbose(void);
extern void add_taint(unsigned flag, enum lockdep_ok);
extern int test_taint(unsigned flag);
extern unsigned long get_taint(void);
diff --git a/kernel/panic.c b/kernel/panic.c
index e1f87ba51ba1..ed270d3b5f2b 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -477,6 +477,7 @@ EXPORT_SYMBOL(panic);
[ TAINT_##taint ] = { \
.c_true = _c_true, .c_false = _c_false, \
.module = _module, \
+ .desc = #taint, \
}

/*
@@ -507,8 +508,9 @@ const struct taint_flag taint_flags[TAINT_FLAGS_COUNT] = {

#undef TAINT_FLAG

-static void print_tainted_seq(struct seq_buf *s)
+static void print_tainted_seq(struct seq_buf *s, bool verbose)
{
+ const char *sep = "";
int i;

if (!tainted_mask) {
@@ -522,10 +524,32 @@ static void print_tainted_seq(struct seq_buf *s)
bool is_set = test_bit(i, &tainted_mask);
char c = is_set ? t->c_true : t->c_false;

- seq_buf_putc(s, c);
+ if (verbose) {
+ if (is_set) {
+ seq_buf_printf(s, "%s[%c]=%s", sep, c, t->desc);
+ sep = ", ";
+ }
+ } else {
+ seq_buf_putc(s, c);
+ }
}
}

+static const char *_print_tainted(bool verbose)
+{
+ /* FIXME: what should the size be? */
+ static char buf[sizeof(taint_flags)];
+ struct seq_buf s;
+
+ BUILD_BUG_ON(ARRAY_SIZE(taint_flags) != TAINT_FLAGS_COUNT);
+
+ seq_buf_init(&s, buf, sizeof(buf));
+
+ print_tainted_seq(&s, verbose);
+
+ return seq_buf_str(&s);
+}
+
/**
* print_tainted - return a string to represent the kernel taint state.
*
@@ -536,16 +560,15 @@ static void print_tainted_seq(struct seq_buf *s)
*/
const char *print_tainted(void)
{
- static char buf[TAINT_FLAGS_COUNT + sizeof("Tainted: ")];
- struct seq_buf s;
-
- BUILD_BUG_ON(ARRAY_SIZE(taint_flags) != TAINT_FLAGS_COUNT);
-
- seq_buf_init(&s, buf, sizeof(buf));
-
- print_tainted_seq(&s);
+ return _print_tainted(false);
+}

- return seq_buf_str(&s);
+/**
+ * print_tainted_verbose - A more verbose version of print_tainted()
+ */
+const char *print_tainted_verbose(void)
+{
+ return _print_tainted(true);
}

int test_taint(unsigned flag)
diff --git a/lib/dump_stack.c b/lib/dump_stack.c
index 222c6d6c8281..8b6b70eaf949 100644
--- a/lib/dump_stack.c
+++ b/lib/dump_stack.c
@@ -62,6 +62,9 @@ void dump_stack_print_info(const char *log_lvl)
(int)strcspn(init_utsname()->version, " "),
init_utsname()->version, BUILD_ID_VAL);

+ if (get_taint())
+ printk("%s%s\n", log_lvl, print_tainted_verbose());
+
if (dump_stack_arch_desc_str[0] != '\0')
printk("%sHardware name: %s\n",
log_lvl, dump_stack_arch_desc_str);
--
2.39.2


2024-03-25 17:20:15

by Jani Nikula

[permalink] [raw]
Subject: [RFC 3/4] kernel/panic: initialize taint_flags[] using a macro

Make it easier to extend struct taint_flags in follow-up.

Cc: Andrew Morton <[email protected]>
Cc: Greg KH <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
---
kernel/panic.c | 46 +++++++++++++++++++++++++++-------------------
1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/kernel/panic.c b/kernel/panic.c
index d2d5f0a4b514..e1f87ba51ba1 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -473,32 +473,40 @@ void panic(const char *fmt, ...)

EXPORT_SYMBOL(panic);

+#define TAINT_FLAG(taint, _c_true, _c_false, _module) \
+ [ TAINT_##taint ] = { \
+ .c_true = _c_true, .c_false = _c_false, \
+ .module = _module, \
+ }
+
/*
* 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] = {
- [ TAINT_PROPRIETARY_MODULE ] = { 'P', 'G', true },
- [ TAINT_FORCED_MODULE ] = { 'F', ' ', true },
- [ TAINT_CPU_OUT_OF_SPEC ] = { 'S', ' ', false },
- [ TAINT_FORCED_RMMOD ] = { 'R', ' ', false },
- [ TAINT_MACHINE_CHECK ] = { 'M', ' ', false },
- [ TAINT_BAD_PAGE ] = { 'B', ' ', false },
- [ TAINT_USER ] = { 'U', ' ', false },
- [ TAINT_DIE ] = { 'D', ' ', false },
- [ TAINT_OVERRIDDEN_ACPI_TABLE ] = { 'A', ' ', false },
- [ TAINT_WARN ] = { 'W', ' ', false },
- [ TAINT_CRAP ] = { 'C', ' ', true },
- [ TAINT_FIRMWARE_WORKAROUND ] = { 'I', ' ', false },
- [ TAINT_OOT_MODULE ] = { 'O', ' ', true },
- [ TAINT_UNSIGNED_MODULE ] = { 'E', ' ', true },
- [ TAINT_SOFTLOCKUP ] = { 'L', ' ', false },
- [ TAINT_LIVEPATCH ] = { 'K', ' ', true },
- [ TAINT_AUX ] = { 'X', ' ', true },
- [ TAINT_RANDSTRUCT ] = { 'T', ' ', true },
- [ TAINT_TEST ] = { 'N', ' ', true },
+ TAINT_FLAG(PROPRIETARY_MODULE, 'P', 'G', true),
+ TAINT_FLAG(FORCED_MODULE, 'F', ' ', true),
+ TAINT_FLAG(CPU_OUT_OF_SPEC, 'S', ' ', false),
+ TAINT_FLAG(FORCED_RMMOD, 'R', ' ', false),
+ TAINT_FLAG(MACHINE_CHECK, 'M', ' ', false),
+ TAINT_FLAG(BAD_PAGE, 'B', ' ', false),
+ TAINT_FLAG(USER, 'U', ' ', false),
+ TAINT_FLAG(DIE, 'D', ' ', false),
+ TAINT_FLAG(OVERRIDDEN_ACPI_TABLE, 'A', ' ', false),
+ TAINT_FLAG(WARN, 'W', ' ', false),
+ TAINT_FLAG(CRAP, 'C', ' ', true),
+ TAINT_FLAG(FIRMWARE_WORKAROUND, 'I', ' ', false),
+ TAINT_FLAG(OOT_MODULE, 'O', ' ', true),
+ TAINT_FLAG(UNSIGNED_MODULE, 'E', ' ', true),
+ TAINT_FLAG(SOFTLOCKUP, 'L', ' ', false),
+ TAINT_FLAG(LIVEPATCH, 'K', ' ', true),
+ TAINT_FLAG(AUX, 'X', ' ', true),
+ TAINT_FLAG(RANDSTRUCT, 'T', ' ', true),
+ TAINT_FLAG(TEST, 'N', ' ', true),
};

+#undef TAINT_FLAG
+
static void print_tainted_seq(struct seq_buf *s)
{
int i;
--
2.39.2