linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] debug: fix WARN_ON_ONCE() for modules
@ 2017-07-15  5:10 Josh Poimboeuf
  2017-07-15  6:29 ` Mike Galbraith
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Josh Poimboeuf @ 2017-07-15  5:10 UTC (permalink / raw)
  To: x86; +Cc: linux-kernel, Mike Galbraith, Peter Zijlstra

Mike Galbraith reported a situation where a WARN_ON_ONCE() call in DRM
code turned into an oops.  As it turns out, WARN_ON_ONCE() seems to be
completely broken when called from a module.

The bug was introduced with the following commit:

  19d436268dde ("debug: Add _ONCE() logic to report_bug()")

That commit changed WARN_ON_ONCE() to move its 'once' logic into the bug
trap handler.  It requires a writable bug table so that the BUGFLAG_DONE
bit can be written to the flags to indicate the first warning has
occurred.

The bug table was made writable for vmlinux, which relies on
vmlinux.lds.S and vmlinux.lds.h for laying out the sections.  However,
it wasn't made writable for modules, which rely on the ELF section
header flags.

Reported-by: Mike Galbraith <efault@gmx.de>
Fixes: 19d436268dde ("debug: Add _ONCE() logic to report_bug()")
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/arm/include/asm/bug.h      | 2 +-
 arch/arm64/include/asm/bug.h    | 2 +-
 arch/blackfin/include/asm/bug.h | 4 ++--
 arch/mn10300/include/asm/bug.h  | 2 +-
 arch/parisc/include/asm/bug.h   | 6 +++---
 arch/powerpc/include/asm/bug.h  | 8 ++++----
 arch/s390/include/asm/bug.h     | 4 ++--
 arch/sh/include/asm/bug.h       | 4 ++--
 arch/x86/include/asm/bug.h      | 4 ++--
 9 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/arch/arm/include/asm/bug.h b/arch/arm/include/asm/bug.h
index 4e6e88a..2244a94e 100644
--- a/arch/arm/include/asm/bug.h
+++ b/arch/arm/include/asm/bug.h
@@ -37,7 +37,7 @@ do {								\
 		".pushsection .rodata.str, \"aMS\", %progbits, 1\n" \
 		"2:\t.asciz " #__file "\n" 			\
 		".popsection\n" 				\
-		".pushsection __bug_table,\"a\"\n"		\
+		".pushsection __bug_table,\"aw\"\n"		\
 		".align 2\n"					\
 		"3:\t.word 1b, 2b\n"				\
 		"\t.hword " #__line ", 0\n"			\
diff --git a/arch/arm64/include/asm/bug.h b/arch/arm64/include/asm/bug.h
index 366448e..a02a571 100644
--- a/arch/arm64/include/asm/bug.h
+++ b/arch/arm64/include/asm/bug.h
@@ -36,7 +36,7 @@
 #ifdef CONFIG_GENERIC_BUG
 
 #define __BUG_ENTRY(flags) 				\
-		".pushsection __bug_table,\"a\"\n\t"	\
+		".pushsection __bug_table,\"aw\"\n\t"	\
 		".align 2\n\t"				\
 	"0:	.long 1f - 0b\n\t"			\
 _BUGVERBOSE_LOCATION(__FILE__, __LINE__)		\
diff --git a/arch/blackfin/include/asm/bug.h b/arch/blackfin/include/asm/bug.h
index 8d9b1eb..76b2e82 100644
--- a/arch/blackfin/include/asm/bug.h
+++ b/arch/blackfin/include/asm/bug.h
@@ -21,7 +21,7 @@
 #define _BUG_OR_WARN(flags)						\
 	asm volatile(							\
 		"1:	.hword	%0\n"					\
-		"	.section __bug_table,\"a\",@progbits\n"		\
+		"	.section __bug_table,\"aw\",@progbits\n"	\
 		"2:	.long	1b\n"					\
 		"	.long	%1\n"					\
 		"	.short	%2\n"					\
@@ -38,7 +38,7 @@
 #define _BUG_OR_WARN(flags)						\
 	asm volatile(							\
 		"1:	.hword	%0\n"					\
-		"	.section __bug_table,\"a\",@progbits\n"		\
+		"	.section __bug_table,\"aw\",@progbits\n"	\
 		"2:	.long	1b\n"					\
 		"	.short	%1\n"					\
 		"	.org	2b + %2\n"				\
diff --git a/arch/mn10300/include/asm/bug.h b/arch/mn10300/include/asm/bug.h
index aa6a388..811414f 100644
--- a/arch/mn10300/include/asm/bug.h
+++ b/arch/mn10300/include/asm/bug.h
@@ -21,7 +21,7 @@ do {								\
 	asm volatile(						\
 		"	syscall 15			\n"	\
 		"0:					\n"	\
-		"	.section __bug_table,\"a\"	\n"	\
+		"	.section __bug_table,\"aw\"	\n"	\
 		"	.long 0b,%0,%1			\n"	\
 		"	.previous			\n"	\
 		:						\
diff --git a/arch/parisc/include/asm/bug.h b/arch/parisc/include/asm/bug.h
index d274227..07ea467 100644
--- a/arch/parisc/include/asm/bug.h
+++ b/arch/parisc/include/asm/bug.h
@@ -27,7 +27,7 @@
 	do {								\
 		asm volatile("\n"					\
 			     "1:\t" PARISC_BUG_BREAK_ASM "\n"		\
-			     "\t.pushsection __bug_table,\"a\"\n"	\
+			     "\t.pushsection __bug_table,\"aw\"\n"	\
 			     "2:\t" ASM_WORD_INSN "1b, %c0\n"		\
 			     "\t.short %c1, %c2\n"			\
 			     "\t.org 2b+%c3\n"				\
@@ -50,7 +50,7 @@
 	do {								\
 		asm volatile("\n"					\
 			     "1:\t" PARISC_BUG_BREAK_ASM "\n"		\
-			     "\t.pushsection __bug_table,\"a\"\n"	\
+			     "\t.pushsection __bug_table,\"aw\"\n"	\
 			     "2:\t" ASM_WORD_INSN "1b, %c0\n"		\
 			     "\t.short %c1, %c2\n"			\
 			     "\t.org 2b+%c3\n"				\
@@ -64,7 +64,7 @@
 	do {								\
 		asm volatile("\n"					\
 			     "1:\t" PARISC_BUG_BREAK_ASM "\n"		\
-			     "\t.pushsection __bug_table,\"a\"\n"	\
+			     "\t.pushsection __bug_table,\"aw\"\n"	\
 			     "2:\t" ASM_WORD_INSN "1b\n"		\
 			     "\t.short %c0\n"				\
 			     "\t.org 2b+%c1\n"				\
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index 0151af6..87fcc19 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -18,7 +18,7 @@
 #include <asm/asm-offsets.h>
 #ifdef CONFIG_DEBUG_BUGVERBOSE
 .macro EMIT_BUG_ENTRY addr,file,line,flags
-	 .section __bug_table,"a"
+	 .section __bug_table,"aw"
 5001:	 PPC_LONG \addr, 5002f
 	 .short \line, \flags
 	 .org 5001b+BUG_ENTRY_SIZE
@@ -29,7 +29,7 @@
 .endm
 #else
 .macro EMIT_BUG_ENTRY addr,file,line,flags
-	 .section __bug_table,"a"
+	 .section __bug_table,"aw"
 5001:	 PPC_LONG \addr
 	 .short \flags
 	 .org 5001b+BUG_ENTRY_SIZE
@@ -42,14 +42,14 @@
    sizeof(struct bug_entry), respectively */
 #ifdef CONFIG_DEBUG_BUGVERBOSE
 #define _EMIT_BUG_ENTRY				\
-	".section __bug_table,\"a\"\n"		\
+	".section __bug_table,\"aw\"\n"		\
 	"2:\t" PPC_LONG "1b, %0\n"		\
 	"\t.short %1, %2\n"			\
 	".org 2b+%3\n"				\
 	".previous\n"
 #else
 #define _EMIT_BUG_ENTRY				\
-	".section __bug_table,\"a\"\n"		\
+	".section __bug_table,\"aw\"\n"		\
 	"2:\t" PPC_LONG "1b\n"			\
 	"\t.short %2\n"				\
 	".org 2b+%3\n"				\
diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h
index 1bbd9db..ce9cc12 100644
--- a/arch/s390/include/asm/bug.h
+++ b/arch/s390/include/asm/bug.h
@@ -14,7 +14,7 @@
 		".section .rodata.str,\"aMS\",@progbits,1\n"	\
 		"2:	.asciz	\""__FILE__"\"\n"		\
 		".previous\n"					\
-		".section __bug_table,\"a\"\n"			\
+		".section __bug_table,\"aw\"\n"			\
 		"3:	.long	1b-3b,2b-3b\n"			\
 		"	.short	%0,%1\n"			\
 		"	.org	3b+%2\n"			\
@@ -30,7 +30,7 @@
 	asm volatile(					\
 		"0:	j	0b+2\n"			\
 		"1:\n"					\
-		".section __bug_table,\"a\"\n"		\
+		".section __bug_table,\"aw\"\n"		\
 		"2:	.long	1b-2b\n"		\
 		"	.short	%0\n"			\
 		"	.org	2b+%1\n"		\
diff --git a/arch/sh/include/asm/bug.h b/arch/sh/include/asm/bug.h
index 1b77f06..986c878 100644
--- a/arch/sh/include/asm/bug.h
+++ b/arch/sh/include/asm/bug.h
@@ -24,14 +24,14 @@
  */
 #ifdef CONFIG_DEBUG_BUGVERBOSE
 #define _EMIT_BUG_ENTRY				\
-	"\t.pushsection __bug_table,\"a\"\n"	\
+	"\t.pushsection __bug_table,\"aw\"\n"	\
 	"2:\t.long 1b, %O1\n"			\
 	"\t.short %O2, %O3\n"			\
 	"\t.org 2b+%O4\n"			\
 	"\t.popsection\n"
 #else
 #define _EMIT_BUG_ENTRY				\
-	"\t.pushsection __bug_table,\"a\"\n"	\
+	"\t.pushsection __bug_table,\"aw\"\n"	\
 	"2:\t.long 1b\n"			\
 	"\t.short %O3\n"			\
 	"\t.org 2b+%O4\n"			\
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index 39e702d..aa6b202 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -35,7 +35,7 @@
 #define _BUG_FLAGS(ins, flags)						\
 do {									\
 	asm volatile("1:\t" ins "\n"					\
-		     ".pushsection __bug_table,\"a\"\n"			\
+		     ".pushsection __bug_table,\"aw\"\n"		\
 		     "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n"	\
 		     "\t"  __BUG_REL(%c0) "\t# bug_entry::file\n"	\
 		     "\t.word %c1"        "\t# bug_entry::line\n"	\
@@ -52,7 +52,7 @@ do {									\
 #define _BUG_FLAGS(ins, flags)						\
 do {									\
 	asm volatile("1:\t" ins "\n"					\
-		     ".pushsection __bug_table,\"a\"\n"			\
+		     ".pushsection __bug_table,\"aw\"\n"		\
 		     "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n"	\
 		     "\t.word %c0"        "\t# bug_entry::flags\n"	\
 		     "\t.org 2b+%c1\n"					\
-- 
2.7.5

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

* Re: [PATCH] debug: fix WARN_ON_ONCE() for modules
  2017-07-15  5:10 [PATCH] debug: fix WARN_ON_ONCE() for modules Josh Poimboeuf
@ 2017-07-15  6:29 ` Mike Galbraith
  2017-07-15 13:26   ` Josh Poimboeuf
  2017-07-16  9:51 ` Masami Hiramatsu
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Mike Galbraith @ 2017-07-15  6:29 UTC (permalink / raw)
  To: Josh Poimboeuf, x86; +Cc: linux-kernel, Peter Zijlstra

On Sat, 2017-07-15 at 00:10 -0500, Josh Poimboeuf wrote:
> 
> The bug table was made writable for vmlinux, which relies on
> vmlinux.lds.S and vmlinux.lds.h for laying out the sections.  However,
> it wasn't made writable for modules, which rely on the ELF section
> header flags.

It seems vmlinux sort of relies on your fix too.  objdump did show
vmlinux __bug_table as being READONLY, but that (lie) is now gone.

	-Mike

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

* Re: [PATCH] debug: fix WARN_ON_ONCE() for modules
  2017-07-15  6:29 ` Mike Galbraith
@ 2017-07-15 13:26   ` Josh Poimboeuf
  0 siblings, 0 replies; 6+ messages in thread
From: Josh Poimboeuf @ 2017-07-15 13:26 UTC (permalink / raw)
  To: Mike Galbraith; +Cc: x86, linux-kernel, Peter Zijlstra

On Sat, Jul 15, 2017 at 08:29:57AM +0200, Mike Galbraith wrote:
> On Sat, 2017-07-15 at 00:10 -0500, Josh Poimboeuf wrote:
> > 
> > The bug table was made writable for vmlinux, which relies on
> > vmlinux.lds.S and vmlinux.lds.h for laying out the sections.  However,
> > it wasn't made writable for modules, which rely on the ELF section
> > header flags.
> 
> It seems vmlinux sort of relies on your fix too.  objdump did show
> vmlinux __bug_table as being READONLY, but that (lie) is now gone.

Yeah, those ELF bits are ignored by the kernel.  It sets its own
protections during boot, e.g. mark_rodata_ro().

-- 
Josh

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

* Re: [PATCH] debug: fix WARN_ON_ONCE() for modules
  2017-07-15  5:10 [PATCH] debug: fix WARN_ON_ONCE() for modules Josh Poimboeuf
  2017-07-15  6:29 ` Mike Galbraith
@ 2017-07-16  9:51 ` Masami Hiramatsu
  2017-07-17 14:00 ` Peter Zijlstra
  2017-07-20 11:42 ` [tip:core/urgent] debug: Fix " tip-bot for Josh Poimboeuf
  3 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2017-07-16  9:51 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: x86, linux-kernel, Mike Galbraith, Peter Zijlstra

On Sat, 15 Jul 2017 00:10:58 -0500
Josh Poimboeuf <jpoimboe@redhat.com> wrote:

> Mike Galbraith reported a situation where a WARN_ON_ONCE() call in DRM
> code turned into an oops.  As it turns out, WARN_ON_ONCE() seems to be
> completely broken when called from a module.
> 
> The bug was introduced with the following commit:
> 
>   19d436268dde ("debug: Add _ONCE() logic to report_bug()")
> 
> That commit changed WARN_ON_ONCE() to move its 'once' logic into the bug
> trap handler.  It requires a writable bug table so that the BUGFLAG_DONE
> bit can be written to the flags to indicate the first warning has
> occurred.
> 
> The bug table was made writable for vmlinux, which relies on
> vmlinux.lds.S and vmlinux.lds.h for laying out the sections.  However,
> it wasn't made writable for modules, which rely on the ELF section
> header flags.

I've hit this problem on 4.12.1 and 4.12.2 and ensured this fixed
that issue. Please push it to stable tree too.

Tested-by: Masami Hiramatsu <mhiramat@kernel.org>

Thank you,

> 
> Reported-by: Mike Galbraith <efault@gmx.de>
> Fixes: 19d436268dde ("debug: Add _ONCE() logic to report_bug()")
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> ---
>  arch/arm/include/asm/bug.h      | 2 +-
>  arch/arm64/include/asm/bug.h    | 2 +-
>  arch/blackfin/include/asm/bug.h | 4 ++--
>  arch/mn10300/include/asm/bug.h  | 2 +-
>  arch/parisc/include/asm/bug.h   | 6 +++---
>  arch/powerpc/include/asm/bug.h  | 8 ++++----
>  arch/s390/include/asm/bug.h     | 4 ++--
>  arch/sh/include/asm/bug.h       | 4 ++--
>  arch/x86/include/asm/bug.h      | 4 ++--
>  9 files changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/arm/include/asm/bug.h b/arch/arm/include/asm/bug.h
> index 4e6e88a..2244a94e 100644
> --- a/arch/arm/include/asm/bug.h
> +++ b/arch/arm/include/asm/bug.h
> @@ -37,7 +37,7 @@ do {								\
>  		".pushsection .rodata.str, \"aMS\", %progbits, 1\n" \
>  		"2:\t.asciz " #__file "\n" 			\
>  		".popsection\n" 				\
> -		".pushsection __bug_table,\"a\"\n"		\
> +		".pushsection __bug_table,\"aw\"\n"		\
>  		".align 2\n"					\
>  		"3:\t.word 1b, 2b\n"				\
>  		"\t.hword " #__line ", 0\n"			\
> diff --git a/arch/arm64/include/asm/bug.h b/arch/arm64/include/asm/bug.h
> index 366448e..a02a571 100644
> --- a/arch/arm64/include/asm/bug.h
> +++ b/arch/arm64/include/asm/bug.h
> @@ -36,7 +36,7 @@
>  #ifdef CONFIG_GENERIC_BUG
>  
>  #define __BUG_ENTRY(flags) 				\
> -		".pushsection __bug_table,\"a\"\n\t"	\
> +		".pushsection __bug_table,\"aw\"\n\t"	\
>  		".align 2\n\t"				\
>  	"0:	.long 1f - 0b\n\t"			\
>  _BUGVERBOSE_LOCATION(__FILE__, __LINE__)		\
> diff --git a/arch/blackfin/include/asm/bug.h b/arch/blackfin/include/asm/bug.h
> index 8d9b1eb..76b2e82 100644
> --- a/arch/blackfin/include/asm/bug.h
> +++ b/arch/blackfin/include/asm/bug.h
> @@ -21,7 +21,7 @@
>  #define _BUG_OR_WARN(flags)						\
>  	asm volatile(							\
>  		"1:	.hword	%0\n"					\
> -		"	.section __bug_table,\"a\",@progbits\n"		\
> +		"	.section __bug_table,\"aw\",@progbits\n"	\
>  		"2:	.long	1b\n"					\
>  		"	.long	%1\n"					\
>  		"	.short	%2\n"					\
> @@ -38,7 +38,7 @@
>  #define _BUG_OR_WARN(flags)						\
>  	asm volatile(							\
>  		"1:	.hword	%0\n"					\
> -		"	.section __bug_table,\"a\",@progbits\n"		\
> +		"	.section __bug_table,\"aw\",@progbits\n"	\
>  		"2:	.long	1b\n"					\
>  		"	.short	%1\n"					\
>  		"	.org	2b + %2\n"				\
> diff --git a/arch/mn10300/include/asm/bug.h b/arch/mn10300/include/asm/bug.h
> index aa6a388..811414f 100644
> --- a/arch/mn10300/include/asm/bug.h
> +++ b/arch/mn10300/include/asm/bug.h
> @@ -21,7 +21,7 @@ do {								\
>  	asm volatile(						\
>  		"	syscall 15			\n"	\
>  		"0:					\n"	\
> -		"	.section __bug_table,\"a\"	\n"	\
> +		"	.section __bug_table,\"aw\"	\n"	\
>  		"	.long 0b,%0,%1			\n"	\
>  		"	.previous			\n"	\
>  		:						\
> diff --git a/arch/parisc/include/asm/bug.h b/arch/parisc/include/asm/bug.h
> index d274227..07ea467 100644
> --- a/arch/parisc/include/asm/bug.h
> +++ b/arch/parisc/include/asm/bug.h
> @@ -27,7 +27,7 @@
>  	do {								\
>  		asm volatile("\n"					\
>  			     "1:\t" PARISC_BUG_BREAK_ASM "\n"		\
> -			     "\t.pushsection __bug_table,\"a\"\n"	\
> +			     "\t.pushsection __bug_table,\"aw\"\n"	\
>  			     "2:\t" ASM_WORD_INSN "1b, %c0\n"		\
>  			     "\t.short %c1, %c2\n"			\
>  			     "\t.org 2b+%c3\n"				\
> @@ -50,7 +50,7 @@
>  	do {								\
>  		asm volatile("\n"					\
>  			     "1:\t" PARISC_BUG_BREAK_ASM "\n"		\
> -			     "\t.pushsection __bug_table,\"a\"\n"	\
> +			     "\t.pushsection __bug_table,\"aw\"\n"	\
>  			     "2:\t" ASM_WORD_INSN "1b, %c0\n"		\
>  			     "\t.short %c1, %c2\n"			\
>  			     "\t.org 2b+%c3\n"				\
> @@ -64,7 +64,7 @@
>  	do {								\
>  		asm volatile("\n"					\
>  			     "1:\t" PARISC_BUG_BREAK_ASM "\n"		\
> -			     "\t.pushsection __bug_table,\"a\"\n"	\
> +			     "\t.pushsection __bug_table,\"aw\"\n"	\
>  			     "2:\t" ASM_WORD_INSN "1b\n"		\
>  			     "\t.short %c0\n"				\
>  			     "\t.org 2b+%c1\n"				\
> diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
> index 0151af6..87fcc19 100644
> --- a/arch/powerpc/include/asm/bug.h
> +++ b/arch/powerpc/include/asm/bug.h
> @@ -18,7 +18,7 @@
>  #include <asm/asm-offsets.h>
>  #ifdef CONFIG_DEBUG_BUGVERBOSE
>  .macro EMIT_BUG_ENTRY addr,file,line,flags
> -	 .section __bug_table,"a"
> +	 .section __bug_table,"aw"
>  5001:	 PPC_LONG \addr, 5002f
>  	 .short \line, \flags
>  	 .org 5001b+BUG_ENTRY_SIZE
> @@ -29,7 +29,7 @@
>  .endm
>  #else
>  .macro EMIT_BUG_ENTRY addr,file,line,flags
> -	 .section __bug_table,"a"
> +	 .section __bug_table,"aw"
>  5001:	 PPC_LONG \addr
>  	 .short \flags
>  	 .org 5001b+BUG_ENTRY_SIZE
> @@ -42,14 +42,14 @@
>     sizeof(struct bug_entry), respectively */
>  #ifdef CONFIG_DEBUG_BUGVERBOSE
>  #define _EMIT_BUG_ENTRY				\
> -	".section __bug_table,\"a\"\n"		\
> +	".section __bug_table,\"aw\"\n"		\
>  	"2:\t" PPC_LONG "1b, %0\n"		\
>  	"\t.short %1, %2\n"			\
>  	".org 2b+%3\n"				\
>  	".previous\n"
>  #else
>  #define _EMIT_BUG_ENTRY				\
> -	".section __bug_table,\"a\"\n"		\
> +	".section __bug_table,\"aw\"\n"		\
>  	"2:\t" PPC_LONG "1b\n"			\
>  	"\t.short %2\n"				\
>  	".org 2b+%3\n"				\
> diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h
> index 1bbd9db..ce9cc12 100644
> --- a/arch/s390/include/asm/bug.h
> +++ b/arch/s390/include/asm/bug.h
> @@ -14,7 +14,7 @@
>  		".section .rodata.str,\"aMS\",@progbits,1\n"	\
>  		"2:	.asciz	\""__FILE__"\"\n"		\
>  		".previous\n"					\
> -		".section __bug_table,\"a\"\n"			\
> +		".section __bug_table,\"aw\"\n"			\
>  		"3:	.long	1b-3b,2b-3b\n"			\
>  		"	.short	%0,%1\n"			\
>  		"	.org	3b+%2\n"			\
> @@ -30,7 +30,7 @@
>  	asm volatile(					\
>  		"0:	j	0b+2\n"			\
>  		"1:\n"					\
> -		".section __bug_table,\"a\"\n"		\
> +		".section __bug_table,\"aw\"\n"		\
>  		"2:	.long	1b-2b\n"		\
>  		"	.short	%0\n"			\
>  		"	.org	2b+%1\n"		\
> diff --git a/arch/sh/include/asm/bug.h b/arch/sh/include/asm/bug.h
> index 1b77f06..986c878 100644
> --- a/arch/sh/include/asm/bug.h
> +++ b/arch/sh/include/asm/bug.h
> @@ -24,14 +24,14 @@
>   */
>  #ifdef CONFIG_DEBUG_BUGVERBOSE
>  #define _EMIT_BUG_ENTRY				\
> -	"\t.pushsection __bug_table,\"a\"\n"	\
> +	"\t.pushsection __bug_table,\"aw\"\n"	\
>  	"2:\t.long 1b, %O1\n"			\
>  	"\t.short %O2, %O3\n"			\
>  	"\t.org 2b+%O4\n"			\
>  	"\t.popsection\n"
>  #else
>  #define _EMIT_BUG_ENTRY				\
> -	"\t.pushsection __bug_table,\"a\"\n"	\
> +	"\t.pushsection __bug_table,\"aw\"\n"	\
>  	"2:\t.long 1b\n"			\
>  	"\t.short %O3\n"			\
>  	"\t.org 2b+%O4\n"			\
> diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
> index 39e702d..aa6b202 100644
> --- a/arch/x86/include/asm/bug.h
> +++ b/arch/x86/include/asm/bug.h
> @@ -35,7 +35,7 @@
>  #define _BUG_FLAGS(ins, flags)						\
>  do {									\
>  	asm volatile("1:\t" ins "\n"					\
> -		     ".pushsection __bug_table,\"a\"\n"			\
> +		     ".pushsection __bug_table,\"aw\"\n"		\
>  		     "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n"	\
>  		     "\t"  __BUG_REL(%c0) "\t# bug_entry::file\n"	\
>  		     "\t.word %c1"        "\t# bug_entry::line\n"	\
> @@ -52,7 +52,7 @@ do {									\
>  #define _BUG_FLAGS(ins, flags)						\
>  do {									\
>  	asm volatile("1:\t" ins "\n"					\
> -		     ".pushsection __bug_table,\"a\"\n"			\
> +		     ".pushsection __bug_table,\"aw\"\n"		\
>  		     "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n"	\
>  		     "\t.word %c0"        "\t# bug_entry::flags\n"	\
>  		     "\t.org 2b+%c1\n"					\
> -- 
> 2.7.5
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

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

* Re: [PATCH] debug: fix WARN_ON_ONCE() for modules
  2017-07-15  5:10 [PATCH] debug: fix WARN_ON_ONCE() for modules Josh Poimboeuf
  2017-07-15  6:29 ` Mike Galbraith
  2017-07-16  9:51 ` Masami Hiramatsu
@ 2017-07-17 14:00 ` Peter Zijlstra
  2017-07-20 11:42 ` [tip:core/urgent] debug: Fix " tip-bot for Josh Poimboeuf
  3 siblings, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2017-07-17 14:00 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: x86, linux-kernel, Mike Galbraith

On Sat, Jul 15, 2017 at 12:10:58AM -0500, Josh Poimboeuf wrote:
> Mike Galbraith reported a situation where a WARN_ON_ONCE() call in DRM
> code turned into an oops.  As it turns out, WARN_ON_ONCE() seems to be
> completely broken when called from a module.
> 
> The bug was introduced with the following commit:
> 
>   19d436268dde ("debug: Add _ONCE() logic to report_bug()")
> 
> That commit changed WARN_ON_ONCE() to move its 'once' logic into the bug
> trap handler.  It requires a writable bug table so that the BUGFLAG_DONE
> bit can be written to the flags to indicate the first warning has
> occurred.
> 
> The bug table was made writable for vmlinux, which relies on
> vmlinux.lds.S and vmlinux.lds.h for laying out the sections.  However,
> it wasn't made writable for modules, which rely on the ELF section
> header flags.
> 
> Reported-by: Mike Galbraith <efault@gmx.de>
> Fixes: 19d436268dde ("debug: Add _ONCE() logic to report_bug()")
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> ---

> diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
> index 39e702d..aa6b202 100644
> --- a/arch/x86/include/asm/bug.h
> +++ b/arch/x86/include/asm/bug.h
> @@ -35,7 +35,7 @@
>  #define _BUG_FLAGS(ins, flags)						\
>  do {									\
>  	asm volatile("1:\t" ins "\n"					\
> -		     ".pushsection __bug_table,\"a\"\n"			\
> +		     ".pushsection __bug_table,\"aw\"\n"		\
>  		     "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n"	\
>  		     "\t"  __BUG_REL(%c0) "\t# bug_entry::file\n"	\
>  		     "\t.word %c1"        "\t# bug_entry::line\n"	\
> @@ -52,7 +52,7 @@ do {									\
>  #define _BUG_FLAGS(ins, flags)						\
>  do {									\
>  	asm volatile("1:\t" ins "\n"					\
> -		     ".pushsection __bug_table,\"a\"\n"			\
> +		     ".pushsection __bug_table,\"aw\"\n"		\
>  		     "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n"	\
>  		     "\t.word %c0"        "\t# bug_entry::flags\n"	\
>  		     "\t.org 2b+%c1\n"					\


Ah shiny. Thanks Josh!

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

* [tip:core/urgent] debug: Fix WARN_ON_ONCE() for modules
  2017-07-15  5:10 [PATCH] debug: fix WARN_ON_ONCE() for modules Josh Poimboeuf
                   ` (2 preceding siblings ...)
  2017-07-17 14:00 ` Peter Zijlstra
@ 2017-07-20 11:42 ` tip-bot for Josh Poimboeuf
  3 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Josh Poimboeuf @ 2017-07-20 11:42 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: efault, peterz, tglx, jpoimboe, linux-kernel, mhiramat, hpa,
	torvalds, mingo

Commit-ID:  325cdacd03c12629aa5f9ee2ace49b1f3dc184a8
Gitweb:     http://git.kernel.org/tip/325cdacd03c12629aa5f9ee2ace49b1f3dc184a8
Author:     Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Sat, 15 Jul 2017 00:10:58 -0500
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 20 Jul 2017 12:31:04 +0200

debug: Fix WARN_ON_ONCE() for modules

Mike Galbraith reported a situation where a WARN_ON_ONCE() call in DRM
code turned into an oops.  As it turns out, WARN_ON_ONCE() seems to be
completely broken when called from a module.

The bug was introduced with the following commit:

  19d436268dde ("debug: Add _ONCE() logic to report_bug()")

That commit changed WARN_ON_ONCE() to move its 'once' logic into the bug
trap handler.  It requires a writable bug table so that the BUGFLAG_DONE
bit can be written to the flags to indicate the first warning has
occurred.

The bug table was made writable for vmlinux, which relies on
vmlinux.lds.S and vmlinux.lds.h for laying out the sections.  However,
it wasn't made writable for modules, which rely on the ELF section
header flags.

Reported-by: Mike Galbraith <efault@gmx.de>
Tested-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Fixes: 19d436268dde ("debug: Add _ONCE() logic to report_bug()")
Link: http://lkml.kernel.org/r/a53b04235a65478dd9afc51f5b329fdc65c84364.1500095401.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/arm/include/asm/bug.h      | 2 +-
 arch/arm64/include/asm/bug.h    | 2 +-
 arch/blackfin/include/asm/bug.h | 4 ++--
 arch/mn10300/include/asm/bug.h  | 2 +-
 arch/parisc/include/asm/bug.h   | 6 +++---
 arch/powerpc/include/asm/bug.h  | 8 ++++----
 arch/s390/include/asm/bug.h     | 4 ++--
 arch/sh/include/asm/bug.h       | 4 ++--
 arch/x86/include/asm/bug.h      | 4 ++--
 9 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/arch/arm/include/asm/bug.h b/arch/arm/include/asm/bug.h
index 4e6e88a..2244a94e 100644
--- a/arch/arm/include/asm/bug.h
+++ b/arch/arm/include/asm/bug.h
@@ -37,7 +37,7 @@ do {								\
 		".pushsection .rodata.str, \"aMS\", %progbits, 1\n" \
 		"2:\t.asciz " #__file "\n" 			\
 		".popsection\n" 				\
-		".pushsection __bug_table,\"a\"\n"		\
+		".pushsection __bug_table,\"aw\"\n"		\
 		".align 2\n"					\
 		"3:\t.word 1b, 2b\n"				\
 		"\t.hword " #__line ", 0\n"			\
diff --git a/arch/arm64/include/asm/bug.h b/arch/arm64/include/asm/bug.h
index 366448e..a02a571 100644
--- a/arch/arm64/include/asm/bug.h
+++ b/arch/arm64/include/asm/bug.h
@@ -36,7 +36,7 @@
 #ifdef CONFIG_GENERIC_BUG
 
 #define __BUG_ENTRY(flags) 				\
-		".pushsection __bug_table,\"a\"\n\t"	\
+		".pushsection __bug_table,\"aw\"\n\t"	\
 		".align 2\n\t"				\
 	"0:	.long 1f - 0b\n\t"			\
 _BUGVERBOSE_LOCATION(__FILE__, __LINE__)		\
diff --git a/arch/blackfin/include/asm/bug.h b/arch/blackfin/include/asm/bug.h
index 8d9b1eb..76b2e82 100644
--- a/arch/blackfin/include/asm/bug.h
+++ b/arch/blackfin/include/asm/bug.h
@@ -21,7 +21,7 @@
 #define _BUG_OR_WARN(flags)						\
 	asm volatile(							\
 		"1:	.hword	%0\n"					\
-		"	.section __bug_table,\"a\",@progbits\n"		\
+		"	.section __bug_table,\"aw\",@progbits\n"	\
 		"2:	.long	1b\n"					\
 		"	.long	%1\n"					\
 		"	.short	%2\n"					\
@@ -38,7 +38,7 @@
 #define _BUG_OR_WARN(flags)						\
 	asm volatile(							\
 		"1:	.hword	%0\n"					\
-		"	.section __bug_table,\"a\",@progbits\n"		\
+		"	.section __bug_table,\"aw\",@progbits\n"	\
 		"2:	.long	1b\n"					\
 		"	.short	%1\n"					\
 		"	.org	2b + %2\n"				\
diff --git a/arch/mn10300/include/asm/bug.h b/arch/mn10300/include/asm/bug.h
index aa6a388..811414f 100644
--- a/arch/mn10300/include/asm/bug.h
+++ b/arch/mn10300/include/asm/bug.h
@@ -21,7 +21,7 @@ do {								\
 	asm volatile(						\
 		"	syscall 15			\n"	\
 		"0:					\n"	\
-		"	.section __bug_table,\"a\"	\n"	\
+		"	.section __bug_table,\"aw\"	\n"	\
 		"	.long 0b,%0,%1			\n"	\
 		"	.previous			\n"	\
 		:						\
diff --git a/arch/parisc/include/asm/bug.h b/arch/parisc/include/asm/bug.h
index d274227..07ea467 100644
--- a/arch/parisc/include/asm/bug.h
+++ b/arch/parisc/include/asm/bug.h
@@ -27,7 +27,7 @@
 	do {								\
 		asm volatile("\n"					\
 			     "1:\t" PARISC_BUG_BREAK_ASM "\n"		\
-			     "\t.pushsection __bug_table,\"a\"\n"	\
+			     "\t.pushsection __bug_table,\"aw\"\n"	\
 			     "2:\t" ASM_WORD_INSN "1b, %c0\n"		\
 			     "\t.short %c1, %c2\n"			\
 			     "\t.org 2b+%c3\n"				\
@@ -50,7 +50,7 @@
 	do {								\
 		asm volatile("\n"					\
 			     "1:\t" PARISC_BUG_BREAK_ASM "\n"		\
-			     "\t.pushsection __bug_table,\"a\"\n"	\
+			     "\t.pushsection __bug_table,\"aw\"\n"	\
 			     "2:\t" ASM_WORD_INSN "1b, %c0\n"		\
 			     "\t.short %c1, %c2\n"			\
 			     "\t.org 2b+%c3\n"				\
@@ -64,7 +64,7 @@
 	do {								\
 		asm volatile("\n"					\
 			     "1:\t" PARISC_BUG_BREAK_ASM "\n"		\
-			     "\t.pushsection __bug_table,\"a\"\n"	\
+			     "\t.pushsection __bug_table,\"aw\"\n"	\
 			     "2:\t" ASM_WORD_INSN "1b\n"		\
 			     "\t.short %c0\n"				\
 			     "\t.org 2b+%c1\n"				\
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index 0151af6..87fcc19 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -18,7 +18,7 @@
 #include <asm/asm-offsets.h>
 #ifdef CONFIG_DEBUG_BUGVERBOSE
 .macro EMIT_BUG_ENTRY addr,file,line,flags
-	 .section __bug_table,"a"
+	 .section __bug_table,"aw"
 5001:	 PPC_LONG \addr, 5002f
 	 .short \line, \flags
 	 .org 5001b+BUG_ENTRY_SIZE
@@ -29,7 +29,7 @@
 .endm
 #else
 .macro EMIT_BUG_ENTRY addr,file,line,flags
-	 .section __bug_table,"a"
+	 .section __bug_table,"aw"
 5001:	 PPC_LONG \addr
 	 .short \flags
 	 .org 5001b+BUG_ENTRY_SIZE
@@ -42,14 +42,14 @@
    sizeof(struct bug_entry), respectively */
 #ifdef CONFIG_DEBUG_BUGVERBOSE
 #define _EMIT_BUG_ENTRY				\
-	".section __bug_table,\"a\"\n"		\
+	".section __bug_table,\"aw\"\n"		\
 	"2:\t" PPC_LONG "1b, %0\n"		\
 	"\t.short %1, %2\n"			\
 	".org 2b+%3\n"				\
 	".previous\n"
 #else
 #define _EMIT_BUG_ENTRY				\
-	".section __bug_table,\"a\"\n"		\
+	".section __bug_table,\"aw\"\n"		\
 	"2:\t" PPC_LONG "1b\n"			\
 	"\t.short %2\n"				\
 	".org 2b+%3\n"				\
diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h
index 1bbd9db..ce9cc12 100644
--- a/arch/s390/include/asm/bug.h
+++ b/arch/s390/include/asm/bug.h
@@ -14,7 +14,7 @@
 		".section .rodata.str,\"aMS\",@progbits,1\n"	\
 		"2:	.asciz	\""__FILE__"\"\n"		\
 		".previous\n"					\
-		".section __bug_table,\"a\"\n"			\
+		".section __bug_table,\"aw\"\n"			\
 		"3:	.long	1b-3b,2b-3b\n"			\
 		"	.short	%0,%1\n"			\
 		"	.org	3b+%2\n"			\
@@ -30,7 +30,7 @@
 	asm volatile(					\
 		"0:	j	0b+2\n"			\
 		"1:\n"					\
-		".section __bug_table,\"a\"\n"		\
+		".section __bug_table,\"aw\"\n"		\
 		"2:	.long	1b-2b\n"		\
 		"	.short	%0\n"			\
 		"	.org	2b+%1\n"		\
diff --git a/arch/sh/include/asm/bug.h b/arch/sh/include/asm/bug.h
index 1b77f06..986c878 100644
--- a/arch/sh/include/asm/bug.h
+++ b/arch/sh/include/asm/bug.h
@@ -24,14 +24,14 @@
  */
 #ifdef CONFIG_DEBUG_BUGVERBOSE
 #define _EMIT_BUG_ENTRY				\
-	"\t.pushsection __bug_table,\"a\"\n"	\
+	"\t.pushsection __bug_table,\"aw\"\n"	\
 	"2:\t.long 1b, %O1\n"			\
 	"\t.short %O2, %O3\n"			\
 	"\t.org 2b+%O4\n"			\
 	"\t.popsection\n"
 #else
 #define _EMIT_BUG_ENTRY				\
-	"\t.pushsection __bug_table,\"a\"\n"	\
+	"\t.pushsection __bug_table,\"aw\"\n"	\
 	"2:\t.long 1b\n"			\
 	"\t.short %O3\n"			\
 	"\t.org 2b+%O4\n"			\
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index 39e702d..aa6b202 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -35,7 +35,7 @@
 #define _BUG_FLAGS(ins, flags)						\
 do {									\
 	asm volatile("1:\t" ins "\n"					\
-		     ".pushsection __bug_table,\"a\"\n"			\
+		     ".pushsection __bug_table,\"aw\"\n"		\
 		     "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n"	\
 		     "\t"  __BUG_REL(%c0) "\t# bug_entry::file\n"	\
 		     "\t.word %c1"        "\t# bug_entry::line\n"	\
@@ -52,7 +52,7 @@ do {									\
 #define _BUG_FLAGS(ins, flags)						\
 do {									\
 	asm volatile("1:\t" ins "\n"					\
-		     ".pushsection __bug_table,\"a\"\n"			\
+		     ".pushsection __bug_table,\"aw\"\n"		\
 		     "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n"	\
 		     "\t.word %c0"        "\t# bug_entry::flags\n"	\
 		     "\t.org 2b+%c1\n"					\

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

end of thread, other threads:[~2017-07-20 11:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-15  5:10 [PATCH] debug: fix WARN_ON_ONCE() for modules Josh Poimboeuf
2017-07-15  6:29 ` Mike Galbraith
2017-07-15 13:26   ` Josh Poimboeuf
2017-07-16  9:51 ` Masami Hiramatsu
2017-07-17 14:00 ` Peter Zijlstra
2017-07-20 11:42 ` [tip:core/urgent] debug: Fix " tip-bot for Josh Poimboeuf

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).