All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler
@ 2019-08-20 16:47 Kees Cook
  2019-08-22 22:56   ` Andrew Morton
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Kees Cook @ 2019-08-20 16:47 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Christophe Leroy, Peter Zijlstra, Drew Davenport, Arnd Bergmann,
	Steven Rostedt (VMware),
	Feng Tang, Petr Mladek, Mauro Carvalho Chehab, Borislav Petkov,
	YueHaibing, linux-arch, linux-kernel

The original clean up of "cut here" missed the WARN_ON() case (that
does not have a printk message), which was fixed recently by adding
an explicit printk of "cut here". This had the downside of adding a
printk() to every WARN_ON() caller, which reduces the utility of using
an instruction exception to streamline the resulting code. By making
this a new BUGFLAG, all of these can be removed and "cut here" can be
handled by the exception handler.

This was very pronounced on PowerPC, but the effect can be seen on
x86 as well. The resulting text size of a defconfig build shows some
small savings from this patch:

   text    data     bss     dec     hex filename
19691167        5134320 1646664 26472151        193eed7 vmlinux.before
19676362        5134260 1663048 26473670        193f4c6 vmlinux.after

This change also opens the door for creating something like BUG_MSG(),
where a custom printk() before issuing BUG(), without confusing the "cut
here" line.

Reported-by: Christophe Leroy <christophe.leroy@c-s.fr>
Fixes: Fixes: 6b15f678fb7d ("include/asm-generic/bug.h: fix "cut here" for WARN_ON for __WARN_TAINT architectures")
Signed-off-by: Kees Cook <keescook@chromium.org>
---
v2:
 - rename BUGFLAG_PRINTK to BUGFLAG_NO_CUT_HERE (peterz, christophe)
---
 include/asm-generic/bug.h |  8 +++-----
 lib/bug.c                 | 11 +++++++++--
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 588dd59a5b72..a21e83f8a274 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -10,6 +10,7 @@
 #define BUGFLAG_WARNING		(1 << 0)
 #define BUGFLAG_ONCE		(1 << 1)
 #define BUGFLAG_DONE		(1 << 2)
+#define BUGFLAG_NO_CUT_HERE	(1 << 3)	/* CUT_HERE already sent */
 #define BUGFLAG_TAINT(taint)	((taint) << 8)
 #define BUG_GET_TAINT(bug)	((bug)->flags >> 8)
 #endif
@@ -86,13 +87,10 @@ void warn_slowpath_fmt(const char *file, const int line, unsigned taint,
 	warn_slowpath_fmt(__FILE__, __LINE__, taint, arg)
 #else
 extern __printf(1, 2) void __warn_printk(const char *fmt, ...);
-#define __WARN() do {							\
-		printk(KERN_WARNING CUT_HERE);				\
-		__WARN_FLAGS(BUGFLAG_TAINT(TAINT_WARN));		\
-	} while (0)
+#define __WARN()		__WARN_FLAGS(BUGFLAG_TAINT(TAINT_WARN))
 #define __WARN_printf(taint, arg...) do {				\
 		__warn_printk(arg);					\
-		__WARN_FLAGS(BUGFLAG_TAINT(taint));			\
+		__WARN_FLAGS(BUGFLAG_NO_CUT_HERE | BUGFLAG_TAINT(taint));\
 	} while (0)
 #define WARN_ON_ONCE(condition) ({				\
 	int __ret_warn_on = !!(condition);			\
diff --git a/lib/bug.c b/lib/bug.c
index 1077366f496b..8c98af0bf585 100644
--- a/lib/bug.c
+++ b/lib/bug.c
@@ -181,6 +181,15 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs)
 		}
 	}
 
+	/*
+	 * BUG() and WARN_ON() families don't print a custom debug message
+	 * before triggering the exception handler, so we must add the
+	 * "cut here" line now. WARN() issues its own "cut here" before the
+	 * extra debugging message it writes before triggering the handler.
+	 */
+	if ((bug->flags & BUGFLAG_NO_CUT_HERE) == 0)
+		printk(KERN_DEFAULT CUT_HERE);
+
 	if (warning) {
 		/* this is a WARN_ON rather than BUG/BUG_ON */
 		__warn(file, line, (void *)bugaddr, BUG_GET_TAINT(bug), regs,
@@ -188,8 +197,6 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs)
 		return BUG_TRAP_TYPE_WARN;
 	}
 
-	printk(KERN_DEFAULT CUT_HERE);
-
 	if (file)
 		pr_crit("kernel BUG at %s:%u!\n", file, line);
 	else
-- 
2.17.1


-- 
Kees Cook

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

* Re: [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler
@ 2019-08-22 22:56   ` Andrew Morton
  0 siblings, 0 replies; 12+ messages in thread
From: Andrew Morton @ 2019-08-22 22:56 UTC (permalink / raw)
  To: 20190819234111.9019-8-keescook
  Cc: Kees Cook, Christophe Leroy, Peter Zijlstra, Drew Davenport,
	Arnd Bergmann, Steven Rostedt (VMware),
	Feng Tang, Petr Mladek, Mauro Carvalho Chehab, Borislav Petkov,
	YueHaibing, linux-arch, linux-kernel

On Tue, 20 Aug 2019 09:47:55 -0700 Kees Cook <keescook@chromium.org> wrote:

> Reply-To: 20190819234111.9019-8-keescook@chromium.org

Really?

> Subject: [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler

It's strange to receive a standalone [7/7] patch.

> Date:   Tue, 20 Aug 2019 09:47:55 -0700
> Sender: linux-kernel-owner@vger.kernel.org
> 
> The original clean up of "cut here" missed the WARN_ON() case (that
> does not have a printk message), which was fixed recently by adding
> an explicit printk of "cut here". This had the downside of adding a
> printk() to every WARN_ON() caller, which reduces the utility of using
> an instruction exception to streamline the resulting code. By making
> this a new BUGFLAG, all of these can be removed and "cut here" can be
> handled by the exception handler.
> 
> This was very pronounced on PowerPC, but the effect can be seen on
> x86 as well. The resulting text size of a defconfig build shows some
> small savings from this patch:
> 
>    text    data     bss     dec     hex filename
> 19691167        5134320 1646664 26472151        193eed7 vmlinux.before
> 19676362        5134260 1663048 26473670        193f4c6 vmlinux.after
> 
> This change also opens the door for creating something like BUG_MSG(),
> where a custom printk() before issuing BUG(), without confusing the "cut
> here" line.

I can't get this to apply to anything, so I guess that [1/7]-[6/7]
mattered ;)

> Reported-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Fixes: Fixes: 6b15f678fb7d ("include/asm-generic/bug.h: fix "cut here" for WARN_ON for __WARN_TAINT architectures")

I'm seeing double.

> Signed-off-by: Kees Cook <keescook@chromium.org>


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

* Re: [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler
@ 2019-08-22 22:56   ` Andrew Morton
  0 siblings, 0 replies; 12+ messages in thread
From: Andrew Morton @ 2019-08-22 22:56 UTC (permalink / raw)
  To: 20190819234111.9019-8-keescook
  Cc: Christophe Leroy, Peter Zijlstra, Drew Davenport, Arnd Bergmann,
	Steven Rostedt (VMware),
	Feng Tang, Petr Mladek, Mauro Carvalho Chehab, Borislav Petkov,
	YueHaibing, linux-arch, linux-kernel

On Tue, 20 Aug 2019 09:47:55 -0700 Kees Cook <keescook@chromium.org> wrote:

> Reply-To: 20190819234111.9019-8-keescook@chromium.org

Really?

> Subject: [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler

It's strange to receive a standalone [7/7] patch.

> Date:   Tue, 20 Aug 2019 09:47:55 -0700
> Sender: linux-kernel-owner@vger.kernel.org
> 
> The original clean up of "cut here" missed the WARN_ON() case (that
> does not have a printk message), which was fixed recently by adding
> an explicit printk of "cut here". This had the downside of adding a
> printk() to every WARN_ON() caller, which reduces the utility of using
> an instruction exception to streamline the resulting code. By making
> this a new BUGFLAG, all of these can be removed and "cut here" can be
> handled by the exception handler.
> 
> This was very pronounced on PowerPC, but the effect can be seen on
> x86 as well. The resulting text size of a defconfig build shows some
> small savings from this patch:
> 
>    text    data     bss     dec     hex filename
> 19691167        5134320 1646664 26472151        193eed7 vmlinux.before
> 19676362        5134260 1663048 26473670        193f4c6 vmlinux.after
> 
> This change also opens the door for creating something like BUG_MSG(),
> where a custom printk() before issuing BUG(), without confusing the "cut
> here" line.

I can't get this to apply to anything, so I guess that [1/7]-[6/7]
mattered ;)

> Reported-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Fixes: Fixes: 6b15f678fb7d ("include/asm-generic/bug.h: fix "cut here" for WARN_ON for __WARN_TAINT architectures")

I'm seeing double.

> Signed-off-by: Kees Cook <keescook@chromium.org>

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

* Re: [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler
@ 2019-08-23 14:26     ` Christophe Leroy
  0 siblings, 0 replies; 12+ messages in thread
From: Christophe Leroy @ 2019-08-23 14:26 UTC (permalink / raw)
  To: Andrew Morton, 20190819234111.9019-8-keescook
  Cc: Kees Cook, Peter Zijlstra, Drew Davenport, Arnd Bergmann,
	Steven Rostedt (VMware),
	Feng Tang, Petr Mladek, Mauro Carvalho Chehab, Borislav Petkov,
	YueHaibing, linux-arch, linux-kernel



Le 23/08/2019 à 00:56, Andrew Morton a écrit :
> On Tue, 20 Aug 2019 09:47:55 -0700 Kees Cook <keescook@chromium.org> wrote:
> 
>> Reply-To: 20190819234111.9019-8-keescook@chromium.org
> 
> Really?

That seems correct, that's the "[PATCH 7/7] bug: Move WARN_ON() "cut 
here" into exception handler" from the series at 
https://lkml.org/lkml/2019/8/19/1155


> 
>> Subject: [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler
> 
> It's strange to receive a standalone [7/7] patch.

Iaw the Reply_To, I understand it as an update of the 7th patch of the 
series.

> 
>> Date:   Tue, 20 Aug 2019 09:47:55 -0700
>> Sender: linux-kernel-owner@vger.kernel.org
>>
>> The original clean up of "cut here" missed the WARN_ON() case (that
>> does not have a printk message), which was fixed recently by adding
>> an explicit printk of "cut here". This had the downside of adding a
>> printk() to every WARN_ON() caller, which reduces the utility of using
>> an instruction exception to streamline the resulting code. By making
>> this a new BUGFLAG, all of these can be removed and "cut here" can be
>> handled by the exception handler.
>>
>> This was very pronounced on PowerPC, but the effect can be seen on
>> x86 as well. The resulting text size of a defconfig build shows some
>> small savings from this patch:
>>
>>     text    data     bss     dec     hex filename
>> 19691167        5134320 1646664 26472151        193eed7 vmlinux.before
>> 19676362        5134260 1663048 26473670        193f4c6 vmlinux.after
>>
>> This change also opens the door for creating something like BUG_MSG(),
>> where a custom printk() before issuing BUG(), without confusing the "cut
>> here" line.
> 
> I can't get this to apply to anything, so I guess that [1/7]-[6/7]
> mattered ;)

On my side it applies cleanly on top of patch 1-6 of the series.

Christophe


> 
>> Reported-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> Fixes: Fixes: 6b15f678fb7d ("include/asm-generic/bug.h: fix "cut here" for WARN_ON for __WARN_TAINT architectures")
> 
> I'm seeing double.
> 
>> Signed-off-by: Kees Cook <keescook@chromium.org>

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

* Re: [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler
@ 2019-08-23 14:26     ` Christophe Leroy
  0 siblings, 0 replies; 12+ messages in thread
From: Christophe Leroy @ 2019-08-23 14:26 UTC (permalink / raw)
  To: Andrew Morton, 20190819234111.9019-8-keescook
  Cc: Peter Zijlstra, Drew Davenport, Arnd Bergmann,
	Steven Rostedt (VMware),
	Feng Tang, Petr Mladek, Mauro Carvalho Chehab, Borislav Petkov,
	YueHaibing, linux-arch, linux-kernel



Le 23/08/2019 à 00:56, Andrew Morton a écrit :
> On Tue, 20 Aug 2019 09:47:55 -0700 Kees Cook <keescook@chromium.org> wrote:
> 
>> Reply-To: 20190819234111.9019-8-keescook@chromium.org
> 
> Really?

That seems correct, that's the "[PATCH 7/7] bug: Move WARN_ON() "cut 
here" into exception handler" from the series at 
https://lkml.org/lkml/2019/8/19/1155


> 
>> Subject: [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler
> 
> It's strange to receive a standalone [7/7] patch.

Iaw the Reply_To, I understand it as an update of the 7th patch of the 
series.

> 
>> Date:   Tue, 20 Aug 2019 09:47:55 -0700
>> Sender: linux-kernel-owner@vger.kernel.org
>>
>> The original clean up of "cut here" missed the WARN_ON() case (that
>> does not have a printk message), which was fixed recently by adding
>> an explicit printk of "cut here". This had the downside of adding a
>> printk() to every WARN_ON() caller, which reduces the utility of using
>> an instruction exception to streamline the resulting code. By making
>> this a new BUGFLAG, all of these can be removed and "cut here" can be
>> handled by the exception handler.
>>
>> This was very pronounced on PowerPC, but the effect can be seen on
>> x86 as well. The resulting text size of a defconfig build shows some
>> small savings from this patch:
>>
>>     text    data     bss     dec     hex filename
>> 19691167        5134320 1646664 26472151        193eed7 vmlinux.before
>> 19676362        5134260 1663048 26473670        193f4c6 vmlinux.after
>>
>> This change also opens the door for creating something like BUG_MSG(),
>> where a custom printk() before issuing BUG(), without confusing the "cut
>> here" line.
> 
> I can't get this to apply to anything, so I guess that [1/7]-[6/7]
> mattered ;)

On my side it applies cleanly on top of patch 1-6 of the series.

Christophe


> 
>> Reported-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> Fixes: Fixes: 6b15f678fb7d ("include/asm-generic/bug.h: fix "cut here" for WARN_ON for __WARN_TAINT architectures")
> 
> I'm seeing double.
> 
>> Signed-off-by: Kees Cook <keescook@chromium.org>

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

* Re: [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler
  2019-08-20 16:47 [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler Kees Cook
  2019-08-22 22:56   ` Andrew Morton
@ 2019-08-23 14:56 ` Christophe Leroy
  2019-09-09 16:05 ` Sergey Senozhatsky
  2 siblings, 0 replies; 12+ messages in thread
From: Christophe Leroy @ 2019-08-23 14:56 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Peter Zijlstra, Drew Davenport, Arnd Bergmann,
	Steven Rostedt (VMware),
	Feng Tang, Petr Mladek, Mauro Carvalho Chehab, Borislav Petkov,
	YueHaibing, linux-arch, linux-kernel

In-Reply-To: 20190819234111.9019-8-keescook@chromium.org

Le 20/08/2019 à 18:47, Kees Cook a écrit :
> The original clean up of "cut here" missed the WARN_ON() case (that
> does not have a printk message), which was fixed recently by adding
> an explicit printk of "cut here". This had the downside of adding a
> printk() to every WARN_ON() caller, which reduces the utility of using
> an instruction exception to streamline the resulting code. By making
> this a new BUGFLAG, all of these can be removed and "cut here" can be
> handled by the exception handler.
> 
> This was very pronounced on PowerPC, but the effect can be seen on
> x86 as well. The resulting text size of a defconfig build shows some
> small savings from this patch:
> 
>     text    data     bss     dec     hex filename
> 19691167        5134320 1646664 26472151        193eed7 vmlinux.before
> 19676362        5134260 1663048 26473670        193f4c6 vmlinux.after
> 
> This change also opens the door for creating something like BUG_MSG(),
> where a custom printk() before issuing BUG(), without confusing the "cut
> here" line.
> 
> Reported-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Fixes: Fixes: 6b15f678fb7d ("include/asm-generic/bug.h: fix "cut here" for WARN_ON for __WARN_TAINT architectures")
> Signed-off-by: Kees Cook <keescook@chromium.org>

Tested-by: Christophe Leroy <christophe.leroy@c-s.fr>

> ---
> v2:
>   - rename BUGFLAG_PRINTK to BUGFLAG_NO_CUT_HERE (peterz, christophe)
> ---
>   include/asm-generic/bug.h |  8 +++-----
>   lib/bug.c                 | 11 +++++++++--
>   2 files changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
> index 588dd59a5b72..a21e83f8a274 100644
> --- a/include/asm-generic/bug.h
> +++ b/include/asm-generic/bug.h
> @@ -10,6 +10,7 @@
>   #define BUGFLAG_WARNING		(1 << 0)
>   #define BUGFLAG_ONCE		(1 << 1)
>   #define BUGFLAG_DONE		(1 << 2)
> +#define BUGFLAG_NO_CUT_HERE	(1 << 3)	/* CUT_HERE already sent */
>   #define BUGFLAG_TAINT(taint)	((taint) << 8)
>   #define BUG_GET_TAINT(bug)	((bug)->flags >> 8)
>   #endif
> @@ -86,13 +87,10 @@ void warn_slowpath_fmt(const char *file, const int line, unsigned taint,
>   	warn_slowpath_fmt(__FILE__, __LINE__, taint, arg)
>   #else
>   extern __printf(1, 2) void __warn_printk(const char *fmt, ...);
> -#define __WARN() do {							\
> -		printk(KERN_WARNING CUT_HERE);				\
> -		__WARN_FLAGS(BUGFLAG_TAINT(TAINT_WARN));		\
> -	} while (0)
> +#define __WARN()		__WARN_FLAGS(BUGFLAG_TAINT(TAINT_WARN))
>   #define __WARN_printf(taint, arg...) do {				\
>   		__warn_printk(arg);					\
> -		__WARN_FLAGS(BUGFLAG_TAINT(taint));			\
> +		__WARN_FLAGS(BUGFLAG_NO_CUT_HERE | BUGFLAG_TAINT(taint));\
>   	} while (0)
>   #define WARN_ON_ONCE(condition) ({				\
>   	int __ret_warn_on = !!(condition);			\
> diff --git a/lib/bug.c b/lib/bug.c
> index 1077366f496b..8c98af0bf585 100644
> --- a/lib/bug.c
> +++ b/lib/bug.c
> @@ -181,6 +181,15 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs)
>   		}
>   	}
>   
> +	/*
> +	 * BUG() and WARN_ON() families don't print a custom debug message
> +	 * before triggering the exception handler, so we must add the
> +	 * "cut here" line now. WARN() issues its own "cut here" before the
> +	 * extra debugging message it writes before triggering the handler.
> +	 */
> +	if ((bug->flags & BUGFLAG_NO_CUT_HERE) == 0)
> +		printk(KERN_DEFAULT CUT_HERE);
> +
>   	if (warning) {
>   		/* this is a WARN_ON rather than BUG/BUG_ON */
>   		__warn(file, line, (void *)bugaddr, BUG_GET_TAINT(bug), regs,
> @@ -188,8 +197,6 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs)
>   		return BUG_TRAP_TYPE_WARN;
>   	}
>   
> -	printk(KERN_DEFAULT CUT_HERE);
> -
>   	if (file)
>   		pr_crit("kernel BUG at %s:%u!\n", file, line);
>   	else
> 

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

* Re: [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler
  2019-08-23 14:26     ` Christophe Leroy
  (?)
@ 2019-08-24 19:08     ` Kees Cook
  2019-08-29  4:55       ` Christophe Leroy
  -1 siblings, 1 reply; 12+ messages in thread
From: Kees Cook @ 2019-08-24 19:08 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Andrew Morton, 20190819234111.9019-8-keescook, Peter Zijlstra,
	Drew Davenport, Arnd Bergmann, Steven Rostedt (VMware),
	Feng Tang, Petr Mladek, Mauro Carvalho Chehab, Borislav Petkov,
	YueHaibing, linux-arch, linux-kernel

On Fri, Aug 23, 2019 at 04:26:59PM +0200, Christophe Leroy wrote:
> 
> 
> Le 23/08/2019 à 00:56, Andrew Morton a écrit :
> > On Tue, 20 Aug 2019 09:47:55 -0700 Kees Cook <keescook@chromium.org> wrote:
> > 
> > > Reply-To: 20190819234111.9019-8-keescook@chromium.org
> > 
> > Really?
> 
> That seems correct, that's the "[PATCH 7/7] bug: Move WARN_ON() "cut here"
> into exception handler" from the series at
> https://lkml.org/lkml/2019/8/19/1155
> 
> 
> > 
> > > Subject: [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler
> > 
> > It's strange to receive a standalone [7/7] patch.
> 
> Iaw the Reply_To, I understand it as an update of the 7th patch of the
> series.

Was trying to avoid the churn of resending the identical 1-6 patches
(which are all just refactoring to make 7/7 not a mess).

I can resend the whole series, if that's preferred.

> > > Reported-by: Christophe Leroy <christophe.leroy@c-s.fr>
> > > Fixes: Fixes: 6b15f678fb7d ("include/asm-generic/bug.h: fix "cut here" for WARN_ON for __WARN_TAINT architectures")
> > 
> > I'm seeing double.

Tracking down all these combinations has been tricky, which is why I did
the patch 1-6 refactoring: it makes the call hierarchy much easier to
examine (IMO).

-Kees

-- 
Kees Cook

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

* Re: [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler
  2019-08-24 19:08     ` Kees Cook
@ 2019-08-29  4:55       ` Christophe Leroy
  2019-08-29 16:12         ` Kees Cook
  0 siblings, 1 reply; 12+ messages in thread
From: Christophe Leroy @ 2019-08-29  4:55 UTC (permalink / raw)
  To: Kees Cook
  Cc: Andrew Morton, Peter Zijlstra, Drew Davenport, Arnd Bergmann,
	Steven Rostedt (VMware),
	Feng Tang, Petr Mladek, Mauro Carvalho Chehab, Borislav Petkov,
	YueHaibing, linux-arch, linux-kernel


Le 24/08/2019 à 21:08, Kees Cook a écrit :

Euh ... only received this mail yesterday. Same for the other answer.


> On Fri, Aug 23, 2019 at 04:26:59PM +0200, Christophe Leroy wrote:
>>
>>
>> Le 23/08/2019 à 00:56, Andrew Morton a écrit :
>>> On Tue, 20 Aug 2019 09:47:55 -0700 Kees Cook <keescook@chromium.org> wrote:
>>>
>>>> Reply-To: 20190819234111.9019-8-keescook@chromium.org
>>>
>>> Really?
>>
>> That seems correct, that's the "[PATCH 7/7] bug: Move WARN_ON() "cut here"
>> into exception handler" from the series at
>> https://lkml.org/lkml/2019/8/19/1155
>>
>>
>>>
>>>> Subject: [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler
>>>
>>> It's strange to receive a standalone [7/7] patch.
>>
>> Iaw the Reply_To, I understand it as an update of the 7th patch of the
>> series.
> 
> Was trying to avoid the churn of resending the identical 1-6 patches
> (which are all just refactoring to make 7/7 not a mess).

Yes but Reply-To: means the address we have to use to answer to this email.

I think you wanted to use In-reply-to:

> 
> I can resend the whole series, if that's preferred.

I guess not.

> 
>>>> Reported-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>>> Fixes: Fixes: 6b15f678fb7d ("include/asm-generic/bug.h: fix "cut here" for WARN_ON for __WARN_TAINT architectures")
>>>
>>> I'm seeing double.
> 
> Tracking down all these combinations has been tricky, which is why I did
> the patch 1-6 refactoring: it makes the call hierarchy much easier to
> examine (IMO).

But still, Andrew is seing double ... And me as well :)

Fixes: Fixes:

Christophe

> 
> -Kees
> 

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

* Re: [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler
  2019-08-29  4:55       ` Christophe Leroy
@ 2019-08-29 16:12         ` Kees Cook
  0 siblings, 0 replies; 12+ messages in thread
From: Kees Cook @ 2019-08-29 16:12 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Andrew Morton, Peter Zijlstra, Drew Davenport, Arnd Bergmann,
	Steven Rostedt (VMware),
	Feng Tang, Petr Mladek, Mauro Carvalho Chehab, Borislav Petkov,
	YueHaibing, linux-arch, linux-kernel

On Thu, Aug 29, 2019 at 06:55:59AM +0200, Christophe Leroy wrote:
> Euh ... only received this mail yesterday. Same for the other answer.

Yeah, my outbound mail was busted. :(

> I think you wanted to use In-reply-to:
> [...]
> But still, Andrew is seing double ... And me as well :)
> 
> Fixes: Fixes:

I had a lot of failures in that email. :)

Thank you Andrew for cleaning this up.

-- 
Kees Cook

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

* Re: [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler
  2019-08-20 16:47 [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler Kees Cook
  2019-08-22 22:56   ` Andrew Morton
  2019-08-23 14:56 ` Christophe Leroy
@ 2019-09-09 16:05 ` Sergey Senozhatsky
  2019-09-10  8:59   ` Kees Cook
  2 siblings, 1 reply; 12+ messages in thread
From: Sergey Senozhatsky @ 2019-09-09 16:05 UTC (permalink / raw)
  To: 20190819234111.9019-8-keescook
  Cc: Andrew Morton, Christophe Leroy, Peter Zijlstra, Drew Davenport,
	Arnd Bergmann, Steven Rostedt (VMware),
	Feng Tang, Petr Mladek, Mauro Carvalho Chehab, Borislav Petkov,
	YueHaibing, linux-arch, linux-kernel

On (08/20/19 09:47), Kees Cook wrote:
[..]
> @@ -181,6 +181,15 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs)
>  		}
>  	}
>  
> +	/*
> +	 * BUG() and WARN_ON() families don't print a custom debug message
> +	 * before triggering the exception handler, so we must add the
> +	 * "cut here" line now. WARN() issues its own "cut here" before the
> +	 * extra debugging message it writes before triggering the handler.
> +	 */
> +	if ((bug->flags & BUGFLAG_NO_CUT_HERE) == 0)
> +		printk(KERN_DEFAULT CUT_HERE);

Shouldn't this be pr_warn() or pr_crit()?

	-ss

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

* Re: [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler
  2019-09-09 16:05 ` Sergey Senozhatsky
@ 2019-09-10  8:59   ` Kees Cook
  2019-09-10  9:16     ` Sergey Senozhatsky
  0 siblings, 1 reply; 12+ messages in thread
From: Kees Cook @ 2019-09-10  8:59 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: 20190819234111.9019-8-keescook, Andrew Morton, Christophe Leroy,
	Peter Zijlstra, Drew Davenport, Arnd Bergmann,
	Steven Rostedt (VMware),
	Feng Tang, Petr Mladek, Mauro Carvalho Chehab, Borislav Petkov,
	YueHaibing, linux-arch, linux-kernel

On Tue, Sep 10, 2019 at 01:05:39AM +0900, Sergey Senozhatsky wrote:
> On (08/20/19 09:47), Kees Cook wrote:
> [..]
> > @@ -181,6 +181,15 @@ enum bug_trap_type report_bug(unsigned long bugaddr, struct pt_regs *regs)
> >  		}
> >  	}
> >  
> > +	/*
> > +	 * BUG() and WARN_ON() families don't print a custom debug message
> > +	 * before triggering the exception handler, so we must add the
> > +	 * "cut here" line now. WARN() issues its own "cut here" before the
> > +	 * extra debugging message it writes before triggering the handler.
> > +	 */
> > +	if ((bug->flags & BUGFLAG_NO_CUT_HERE) == 0)
> > +		printk(KERN_DEFAULT CUT_HERE);
> 
> Shouldn't this be pr_warn() or pr_crit()?

The pr_* helpers here would (potentially) add unwanted prefixes, so
those aren't used. KERN_DEFAULT is used here because that's how it's
always been printed. I didn't want to change that for this refactoring
work. I'm not opposed to it, generally speaking, though. :)

-- 
Kees Cook

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

* Re: [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler
  2019-09-10  8:59   ` Kees Cook
@ 2019-09-10  9:16     ` Sergey Senozhatsky
  0 siblings, 0 replies; 12+ messages in thread
From: Sergey Senozhatsky @ 2019-09-10  9:16 UTC (permalink / raw)
  To: Kees Cook
  Cc: Sergey Senozhatsky, Andrew Morton, Christophe Leroy,
	Peter Zijlstra, Drew Davenport, Arnd Bergmann,
	Steven Rostedt (VMware),
	Feng Tang, Petr Mladek, Mauro Carvalho Chehab, Borislav Petkov,
	YueHaibing, linux-arch, linux-kernel

On (09/10/19 01:59), Kees Cook wrote:
> On Tue, Sep 10, 2019 at 01:05:39AM +0900, Sergey Senozhatsky wrote:
> > On (08/20/19 09:47), Kees Cook wrote:
> > [..]
> > > +	/*
> > > +	 * BUG() and WARN_ON() families don't print a custom debug message
> > > +	 * before triggering the exception handler, so we must add the
> > > +	 * "cut here" line now. WARN() issues its own "cut here" before the
> > > +	 * extra debugging message it writes before triggering the handler.
> > > +	 */
> > > +	if ((bug->flags & BUGFLAG_NO_CUT_HERE) == 0)
> > > +		printk(KERN_DEFAULT CUT_HERE);
> > 
> > Shouldn't this be pr_warn() or pr_crit()?
> 
> The pr_* helpers here would (potentially) add unwanted prefixes, so
> those aren't used. KERN_DEFAULT is used here because that's how it's
> always been printed. I didn't want to change that for this refactoring
> work. I'm not opposed to it, generally speaking, though. :)

I just glanced through CUT_HERE users

warn_slowpath_fmt()    pr_warn(CUT_HERE)
__warn_printk()        pr_warn(CUT_HERE)
rdma_restrack_clean()  pr_err("restrack: %s", CUT_HERE)
rdma_restrack_clean()  pr_err("restrack: %s", CUT_HERE)

+ oops/panic end markers are of pr_warn() or pr_crit() log levels.
So I thought that maybe we can make it more or less similar.

But if it has always been this way (KERN_DEFAULT) then OK.

	-ss

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

end of thread, other threads:[~2019-09-10  9:16 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-20 16:47 [PATCH v2 7/7] bug: Move WARN_ON() "cut here" into exception handler Kees Cook
2019-08-22 22:56 ` Andrew Morton
2019-08-22 22:56   ` Andrew Morton
2019-08-23 14:26   ` Christophe Leroy
2019-08-23 14:26     ` Christophe Leroy
2019-08-24 19:08     ` Kees Cook
2019-08-29  4:55       ` Christophe Leroy
2019-08-29 16:12         ` Kees Cook
2019-08-23 14:56 ` Christophe Leroy
2019-09-09 16:05 ` Sergey Senozhatsky
2019-09-10  8:59   ` Kees Cook
2019-09-10  9:16     ` Sergey Senozhatsky

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.