All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] bug: Fix "cut here" location for __WARN_TAINT
@ 2017-11-08  0:27 Kees Cook
  2017-11-08  0:27 ` [PATCH 1/3] lkdtm: Include WARN format string Kees Cook
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Kees Cook @ 2017-11-08  0:27 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Peter Zijlstra, Josh Poimboeuf, Joe Perches,
	Fengguang Wu, Arnd Bergmann, linux-am33-list, linux-kernel,
	linux-arch

Quoting the last patch in the series:

Prior to v4.11, x86 used warn_slowpath_fmt() for handling WARN()s. After
WARN() was moved to using UD0 on x86, the warning text started appearing
_before_ the "cut here" line. This appears to have been a long-standing
bug on architectures that used __WARN_TAINT, but it didn't get fixed.

v4.11 and earlier on x86:

[    7.944142] ------------[ cut here ]------------
[    7.945631] WARNING: CPU: 0 PID: 2956 at drivers/misc/lkdtm_bugs.c:65 lkdtm_WARNING+0x21/0x30
[    7.947453] This is a warning message
[    7.948357] Modules linked in:

v4.12 and later on x86:

[    8.973063] This is a warning message
[    8.973885] ------------[ cut here ]------------
[    8.974867] WARNING: CPU: 1 PID: 2982 at drivers/misc/lkdtm_bugs.c:68 lkdtm_WARNING+0x15/0x20
[    8.976563] Modules linked in:

With this fix:

[    9.157133] ------------[ cut here ]------------
[    9.158143] This is a warning message
[    9.159099] WARNING: CPU: 3 PID: 3009 at drivers/misc/lkdtm_bugs.c:67 lkdtm_WARNING+0x15/0x20

Since the __FILE__ reporting happens as part of the UD0 handler, it isn't
trivial to move the message to after the WARNING line, but at least we can
fix the position of the "cut here" line so all the various logging tools
will start including the actual runtime warning message again, when they
follow the instruction and "cut here".

Fixes: 9a93848fe787 ("x86/debug: Implement __WARN() using UD0")

Thanks!

-Kees

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/3] lkdtm: Include WARN format string
  2017-11-08  0:27 [PATCH 0/3] bug: Fix "cut here" location for __WARN_TAINT Kees Cook
@ 2017-11-08  0:27 ` Kees Cook
  2017-11-08  0:27 ` [PATCH 2/3] bug: Define the "cut here" string in a single place Kees Cook
  2017-11-08  0:27 ` [PATCH 3/3] bug: Fix "cut here" location for __WARN_TAINT architectures Kees Cook
  2 siblings, 0 replies; 4+ messages in thread
From: Kees Cook @ 2017-11-08  0:27 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Peter Zijlstra, Josh Poimboeuf, Joe Perches,
	Fengguang Wu, Arnd Bergmann, linux-am33-list, linux-kernel,
	linux-arch

In order to test the ordering of WARN format strings, actually include one
in LKDTM.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/misc/lkdtm_bugs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/lkdtm_bugs.c b/drivers/misc/lkdtm_bugs.c
index 9e0b4f959987..8523f7dbac99 100644
--- a/drivers/misc/lkdtm_bugs.c
+++ b/drivers/misc/lkdtm_bugs.c
@@ -62,9 +62,11 @@ void lkdtm_BUG(void)
 	BUG();
 }
 
+static int warn_counter;
+
 void lkdtm_WARNING(void)
 {
-	WARN_ON(1);
+	WARN(1, "Warning message trigger count: %d\n", warn_counter++);
 }
 
 void lkdtm_EXCEPTION(void)
-- 
2.7.4

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/3] bug: Define the "cut here" string in a single place
  2017-11-08  0:27 [PATCH 0/3] bug: Fix "cut here" location for __WARN_TAINT Kees Cook
  2017-11-08  0:27 ` [PATCH 1/3] lkdtm: Include WARN format string Kees Cook
@ 2017-11-08  0:27 ` Kees Cook
  2017-11-08  0:27 ` [PATCH 3/3] bug: Fix "cut here" location for __WARN_TAINT architectures Kees Cook
  2 siblings, 0 replies; 4+ messages in thread
From: Kees Cook @ 2017-11-08  0:27 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Peter Zijlstra, Josh Poimboeuf, Joe Perches,
	Fengguang Wu, Arnd Bergmann, linux-am33-list, linux-kernel,
	linux-arch

The "cut here" string is used in a few paths. Define it in a single place.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/mn10300/mm/fault.c   | 2 +-
 include/asm-generic/bug.h | 2 ++
 kernel/panic.c            | 2 +-
 lib/bug.c                 | 2 +-
 4 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/mn10300/mm/fault.c b/arch/mn10300/mm/fault.c
index f23781d6bbb3..f0bfa1448744 100644
--- a/arch/mn10300/mm/fault.c
+++ b/arch/mn10300/mm/fault.c
@@ -60,7 +60,7 @@ void bust_spinlocks(int yes)
 void do_BUG(const char *file, int line)
 {
 	bust_spinlocks(1);
-	printk(KERN_EMERG "------------[ cut here ]------------\n");
+	printk(KERN_EMERG CUT_HERE);
 	printk(KERN_EMERG "kernel BUG at %s:%d!\n", file, line);
 }
 
diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 87191357d303..673a79dd3928 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -3,6 +3,8 @@
 
 #include <linux/compiler.h>
 
+#define CUT_HERE		"------------[ cut here ]------------\n"
+
 #ifdef CONFIG_GENERIC_BUG
 #define BUGFLAG_WARNING		(1 << 0)
 #define BUGFLAG_ONCE		(1 << 1)
diff --git a/kernel/panic.c b/kernel/panic.c
index bdd18afa19a4..ab210714f2f3 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -518,7 +518,7 @@ void __warn(const char *file, int line, void *caller, unsigned taint,
 {
 	disable_trace_on_warning();
 
-	pr_warn("------------[ cut here ]------------\n");
+	pr_warn(CUT_HERE);
 
 	if (file)
 		pr_warn("WARNING: CPU: %d PID: %d at %s:%d %pS\n",
diff --git a/lib/bug.c b/lib/bug.c
index a6a1137d06db..2c9e04621638 100644
--- a/lib/bug.c
+++ b/lib/bug.c
@@ -185,7 +185,7 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs)
 		return BUG_TRAP_TYPE_WARN;
 	}
 
-	printk(KERN_DEFAULT "------------[ cut here ]------------\n");
+	printk(KERN_DEFAULT CUT_HERE);
 
 	if (file)
 		pr_crit("kernel BUG at %s:%u!\n", file, line);
-- 
2.7.4

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 3/3] bug: Fix "cut here" location for __WARN_TAINT architectures
  2017-11-08  0:27 [PATCH 0/3] bug: Fix "cut here" location for __WARN_TAINT Kees Cook
  2017-11-08  0:27 ` [PATCH 1/3] lkdtm: Include WARN format string Kees Cook
  2017-11-08  0:27 ` [PATCH 2/3] bug: Define the "cut here" string in a single place Kees Cook
@ 2017-11-08  0:27 ` Kees Cook
  2 siblings, 0 replies; 4+ messages in thread
From: Kees Cook @ 2017-11-08  0:27 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Peter Zijlstra, Josh Poimboeuf, Fengguang Wu,
	Arnd Bergmann, linux-arch, Joe Perches, linux-am33-list,
	linux-kernel

Prior to v4.11, x86 used warn_slowpath_fmt() for handling WARN()s. After
WARN() was moved to using UD0 on x86, the warning text started appearing
_before_ the "cut here" line. This appears to have been a long-standing
bug on architectures that used __WARN_TAINT, but it didn't get fixed.

v4.11 and earlier on x86:

[    7.944142] ------------[ cut here ]------------
[    7.945631] WARNING: CPU: 0 PID: 2956 at drivers/misc/lkdtm_bugs.c:65 lkdtm_WARNING+0x21/0x30
[    7.947453] This is a warning message
[    7.948357] Modules linked in:

v4.12 and later on x86:

[    8.973063] This is a warning message
[    8.973885] ------------[ cut here ]------------
[    8.974867] WARNING: CPU: 1 PID: 2982 at drivers/misc/lkdtm_bugs.c:68 lkdtm_WARNING+0x15/0x20
[    8.976563] Modules linked in:

With this fix:

[    9.157133] ------------[ cut here ]------------
[    9.158143] This is a warning message
[    9.159099] WARNING: CPU: 3 PID: 3009 at drivers/misc/lkdtm_bugs.c:67 lkdtm_WARNING+0x15/0x20

Since the __FILE__ reporting happens as part of the UD0 handler, it isn't
trivial to move the message to after the WARNING line, but at least we can
fix the position of the "cut here" line so all the various logging tools
will start including the actual runtime warning message again, when they
follow the instruction and "cut here".

Fixes: 9a93848fe787 ("x86/debug: Implement __WARN() using UD0")
Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Fengguang Wu <fengguang.wu@intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: linux-arch@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/asm-generic/bug.h |  5 +++--
 kernel/panic.c            | 16 +++++++++++++++-
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 673a79dd3928..30b22e47a7e9 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -91,10 +91,11 @@ extern void warn_slowpath_null(const char *file, const int line);
 #define __WARN_printf_taint(taint, arg...)				\
 	warn_slowpath_fmt_taint(__FILE__, __LINE__, taint, arg)
 #else
+extern __printf(1, 2) void __warn_printk(const char *fmt, ...);
 #define __WARN()		__WARN_TAINT(TAINT_WARN)
-#define __WARN_printf(arg...)	do { printk(arg); __WARN(); } while (0)
+#define __WARN_printf(arg...)	do { __warn_printk(arg); __WARN(); } while (0)
 #define __WARN_printf_taint(taint, arg...)				\
-	do { printk(arg); __WARN_TAINT(taint); } while (0)
+	do { __warn_printk(arg); __WARN_TAINT(taint); } while (0)
 #endif
 
 /* used internally by panic.c */
diff --git a/kernel/panic.c b/kernel/panic.c
index ab210714f2f3..65c9e7b942ea 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -518,7 +518,8 @@ void __warn(const char *file, int line, void *caller, unsigned taint,
 {
 	disable_trace_on_warning();
 
-	pr_warn(CUT_HERE);
+	if (args)
+		pr_warn(CUT_HERE);
 
 	if (file)
 		pr_warn("WARNING: CPU: %d PID: %d at %s:%d %pS\n",
@@ -582,9 +583,22 @@ EXPORT_SYMBOL(warn_slowpath_fmt_taint);
 
 void warn_slowpath_null(const char *file, int line)
 {
+	pr_warn(CUT_HERE);
 	__warn(file, line, __builtin_return_address(0), TAINT_WARN, NULL, NULL);
 }
 EXPORT_SYMBOL(warn_slowpath_null);
+#else
+void __warn_printk(const char *fmt, ...)
+{
+	va_list args;
+
+	pr_warn(CUT_HERE);
+
+	va_start(args, fmt);
+	vprintk(fmt, args);
+	va_end(args);
+}
+EXPORT_SYMBOL(__warn_printk);
 #endif
 
 #ifdef CONFIG_CC_STACKPROTECTOR
-- 
2.7.4

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-11-08  0:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-08  0:27 [PATCH 0/3] bug: Fix "cut here" location for __WARN_TAINT Kees Cook
2017-11-08  0:27 ` [PATCH 1/3] lkdtm: Include WARN format string Kees Cook
2017-11-08  0:27 ` [PATCH 2/3] bug: Define the "cut here" string in a single place Kees Cook
2017-11-08  0:27 ` [PATCH 3/3] bug: Fix "cut here" location for __WARN_TAINT architectures Kees Cook

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.