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


Groups > linux.kernel > #1612353 > unrolled thread

[PATCH] x86/debug: define BUG() againfor !CONFIG_BUG

Started byArnd Bergmann <arnd@arndb.de>
First post2017-03-29 23:20 +0200
Last post2017-03-30 09:40 +0200
Articles 9 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] x86/debug: define BUG() againfor !CONFIG_BUG Arnd Bergmann <arnd@arndb.de> - 2017-03-29 23:20 +0200
    Re: [PATCH] x86/debug: define BUG() againfor !CONFIG_BUG Peter Zijlstra <peterz@infradead.org> - 2017-03-30 09:20 +0200
      Re: [PATCH] x86/debug: define BUG() againfor !CONFIG_BUG Ingo Molnar <mingo@kernel.org> - 2017-03-30 09:20 +0200
        Re: [PATCH] x86/debug: define BUG() againfor !CONFIG_BUG Ingo Molnar <mingo@kernel.org> - 2017-03-30 09:30 +0200
        Re: [PATCH] x86/debug: define BUG() againfor !CONFIG_BUG Peter Zijlstra <peterz@infradead.org> - 2017-03-30 09:40 +0200
          Re: [PATCH] x86/debug: define BUG() againfor !CONFIG_BUG Ingo Molnar <mingo@kernel.org> - 2017-03-30 09:50 +0200
            Re: [PATCH] x86/debug: define BUG() againfor !CONFIG_BUG Arnd Bergmann <arnd@arndb.de> - 2017-03-30 10:10 +0200
      Re: [PATCH] x86/debug: define BUG() againfor !CONFIG_BUG Arnd Bergmann <arnd@arndb.de> - 2017-03-30 09:30 +0200
    [tip:x86/asm] x86/debug: Define BUG() again for !CONFIG_BUG tip-bot for Arnd Bergmann <tipbot@zytor.com> - 2017-03-30 09:40 +0200

#1612353 — [PATCH] x86/debug: define BUG() againfor !CONFIG_BUG

FromArnd Bergmann <arnd@arndb.de>
Date2017-03-29 23:20 +0200
Subject[PATCH] x86/debug: define BUG() againfor !CONFIG_BUG
Message-ID<tqtay-4hp-25@gated-at.bofh.it>
The latest change to the BUG() macro inadvertently reverted the earlier
commit b06dd879f5db ("x86: always define BUG() and HAVE_ARCH_BUG, even
with !CONFIG_BUG") that sanitized the behavior with CONFIG_BUG=n.

I noticed this as some warnings have appeared again that were previously
fixed as a side effect of that patch:

kernel/seccomp.c: In function '__seccomp_filter':
kernel/seccomp.c:670:1: error: no return statement in function returning non-void [-Werror=return-type]

drivers/gpu/drm/i915/intel_sprite.c: In function 'intel_check_sprite_plane':
drivers/gpu/drm/i915/intel_sprite.c:936:20: error: 'src_h' may be used uninitialized in this function [-Werror=maybe-uninitialized]
   src->y2 = (src_y + src_h) << 16;
             ~~~~~~~^~~~~~~~
drivers/gpu/drm/i915/intel_sprite.c:934:20: error: 'src_w' may be used uninitialized in this function [-Werror=maybe-uninitialized]
   src->x2 = (src_x + src_w) << 16;
             ~~~~~~~^~~~~~~~
drivers/gpu/drm/i915/intel_sprite.c:936:20: error: 'src_y' may be used uninitialized in this function [-Werror=maybe-uninitialized]
   src->y2 = (src_y + src_h) << 16;
             ~~~~~~~^~~~~~~~
drivers/gpu/drm/i915/intel_sprite.c:934:20: error: 'src_x' may be used uninitialized in this function [-Werror=maybe-uninitialized]
   src->x2 = (src_x + src_w) << 16;
             ~~~~~~~^~~~~~~~

This combines the two patches and uses the ud2 macro to define BUG()
in case of CONFIG_BUG=n.

Fixes: 9a93848fe787 ("x86/debug: Implement __WARN() using UD0")
Cc: Josh Triplett <josh@joshtriplett.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/include/asm/bug.h | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index 4fde330c44b7..cecf559d0012 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -23,7 +23,6 @@
 #define LEN_UD0		2
 
 #ifdef CONFIG_GENERIC_BUG
-#define HAVE_ARCH_BUG
 
 #ifdef CONFIG_X86_32
 # define __BUG_REL(val)	".long " __stringify(val)
@@ -64,6 +63,13 @@ do {									\
 
 #endif /* CONFIG_DEBUG_BUGVERBOSE */
 
+#else
+
+#define _BUG_FLAGS(ins, flags)  asm volatile(ins)
+
+#endif /* CONFIG_GENERIC_BUG */
+
+#define HAVE_ARCH_BUG
 #define BUG()							\
 do {								\
 	_BUG_FLAGS(ASM_UD2, 0);					\
@@ -72,8 +78,6 @@ do {								\
 
 #define __WARN_TAINT(taint)	_BUG_FLAGS(ASM_UD0, BUGFLAG_TAINT(taint))
 
-#endif /* CONFIG_GENERIC_BUG */
-
 #include <asm-generic/bug.h>
 
 #endif /* _ASM_X86_BUG_H */
-- 
2.9.0

[toc] | [next] | [standalone]


#1612642

FromPeter Zijlstra <peterz@infradead.org>
Date2017-03-30 09:20 +0200
Message-ID<tqCxb-2Ja-1@gated-at.bofh.it>
In reply to#1612353
On Wed, Mar 29, 2017 at 11:16:31PM +0200, Arnd Bergmann wrote:
> The latest change to the BUG() macro inadvertently reverted the earlier
> commit b06dd879f5db ("x86: always define BUG() and HAVE_ARCH_BUG, even
> with !CONFIG_BUG") that sanitized the behavior with CONFIG_BUG=n.
> 
> I noticed this as some warnings have appeared again that were previously
> fixed as a side effect of that patch:
> 
> kernel/seccomp.c: In function '__seccomp_filter':
> kernel/seccomp.c:670:1: error: no return statement in function returning non-void [-Werror=return-type]
> 
> drivers/gpu/drm/i915/intel_sprite.c: In function 'intel_check_sprite_plane':
> drivers/gpu/drm/i915/intel_sprite.c:936:20: error: 'src_h' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>    src->y2 = (src_y + src_h) << 16;
>              ~~~~~~~^~~~~~~~
> drivers/gpu/drm/i915/intel_sprite.c:934:20: error: 'src_w' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>    src->x2 = (src_x + src_w) << 16;
>              ~~~~~~~^~~~~~~~
> drivers/gpu/drm/i915/intel_sprite.c:936:20: error: 'src_y' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>    src->y2 = (src_y + src_h) << 16;
>              ~~~~~~~^~~~~~~~
> drivers/gpu/drm/i915/intel_sprite.c:934:20: error: 'src_x' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>    src->x2 = (src_x + src_w) << 16;
>              ~~~~~~~^~~~~~~~
> 
> This combines the two patches and uses the ud2 macro to define BUG()
> in case of CONFIG_BUG=n.

OK, fair enough I suppose. However, I cribbed this from arm64. What does
that do for BUG=n ?

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


#1612648

FromIngo Molnar <mingo@kernel.org>
Date2017-03-30 09:20 +0200
Message-ID<tqCxc-2Ja-25@gated-at.bofh.it>
In reply to#1612642
* Peter Zijlstra <peterz@infradead.org> wrote:

> On Wed, Mar 29, 2017 at 11:16:31PM +0200, Arnd Bergmann wrote:
> > The latest change to the BUG() macro inadvertently reverted the earlier
> > commit b06dd879f5db ("x86: always define BUG() and HAVE_ARCH_BUG, even
> > with !CONFIG_BUG") that sanitized the behavior with CONFIG_BUG=n.
> > 
> > I noticed this as some warnings have appeared again that were previously
> > fixed as a side effect of that patch:
> > 
> > kernel/seccomp.c: In function '__seccomp_filter':
> > kernel/seccomp.c:670:1: error: no return statement in function returning non-void [-Werror=return-type]
> > 
> > drivers/gpu/drm/i915/intel_sprite.c: In function 'intel_check_sprite_plane':
> > drivers/gpu/drm/i915/intel_sprite.c:936:20: error: 'src_h' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >    src->y2 = (src_y + src_h) << 16;
> >              ~~~~~~~^~~~~~~~
> > drivers/gpu/drm/i915/intel_sprite.c:934:20: error: 'src_w' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >    src->x2 = (src_x + src_w) << 16;
> >              ~~~~~~~^~~~~~~~
> > drivers/gpu/drm/i915/intel_sprite.c:936:20: error: 'src_y' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >    src->y2 = (src_y + src_h) << 16;
> >              ~~~~~~~^~~~~~~~
> > drivers/gpu/drm/i915/intel_sprite.c:934:20: error: 'src_x' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >    src->x2 = (src_x + src_w) << 16;
> >              ~~~~~~~^~~~~~~~
> > 
> > This combines the two patches and uses the ud2 macro to define BUG()
> > in case of CONFIG_BUG=n.
> 
> OK, fair enough I suppose. However, I cribbed this from arm64. What does
> that do for BUG=n ?

I think we'll get a U2D crash in this case, without any bug information.

I.e. only marginally debuggable, but it's a deterministic outcome - instead of the 
crazy GCC code generation variant of the day when the warning triggers, or the 
similarly crazy infinite loop hang.

I'm not entirely sure though, I don't think many people actually _use_ 
CONFIG_BUG=n, it's essentially a crazy thing to do even on constrainted hardware. 
Debugging and maintenance costs almost always trump marginal hardware costs of a 
bit more debugging code.

Thanks,

	Ingo

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


#1612660

FromIngo Molnar <mingo@kernel.org>
Date2017-03-30 09:30 +0200
Message-ID<tqCGS-2MS-15@gated-at.bofh.it>
In reply to#1612648
* Ingo Molnar <mingo@kernel.org> wrote:

> 
> * Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Wed, Mar 29, 2017 at 11:16:31PM +0200, Arnd Bergmann wrote:
> > > The latest change to the BUG() macro inadvertently reverted the earlier
> > > commit b06dd879f5db ("x86: always define BUG() and HAVE_ARCH_BUG, even
> > > with !CONFIG_BUG") that sanitized the behavior with CONFIG_BUG=n.
> > > 
> > > I noticed this as some warnings have appeared again that were previously
> > > fixed as a side effect of that patch:
> > > 
> > > kernel/seccomp.c: In function '__seccomp_filter':
> > > kernel/seccomp.c:670:1: error: no return statement in function returning non-void [-Werror=return-type]
> > > 
> > > drivers/gpu/drm/i915/intel_sprite.c: In function 'intel_check_sprite_plane':
> > > drivers/gpu/drm/i915/intel_sprite.c:936:20: error: 'src_h' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > >    src->y2 = (src_y + src_h) << 16;
> > >              ~~~~~~~^~~~~~~~
> > > drivers/gpu/drm/i915/intel_sprite.c:934:20: error: 'src_w' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > >    src->x2 = (src_x + src_w) << 16;
> > >              ~~~~~~~^~~~~~~~
> > > drivers/gpu/drm/i915/intel_sprite.c:936:20: error: 'src_y' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > >    src->y2 = (src_y + src_h) << 16;
> > >              ~~~~~~~^~~~~~~~
> > > drivers/gpu/drm/i915/intel_sprite.c:934:20: error: 'src_x' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > >    src->x2 = (src_x + src_w) << 16;
> > >              ~~~~~~~^~~~~~~~
> > > 
> > > This combines the two patches and uses the ud2 macro to define BUG()
> > > in case of CONFIG_BUG=n.
> > 
> > OK, fair enough I suppose. However, I cribbed this from arm64. What does
> > that do for BUG=n ?
> 
> I think we'll get a U2D crash in this case, without any bug information.
> 
> I.e. only marginally debuggable, but it's a deterministic outcome - instead of the 
> crazy GCC code generation variant of the day when the warning triggers, or the 
> similarly crazy infinite loop hang.
> 
> I'm not entirely sure though, I don't think many people actually _use_ 
> CONFIG_BUG=n, it's essentially a crazy thing to do even on constrainted hardware. 
> Debugging and maintenance costs almost always trump marginal hardware costs of a 
> bit more debugging code.

So I've applied Arnd's patch to restore the previous behavior - but I agree with 
you that the situation isn't entirely logical at the moment. At minimum we need a 
comment explaining it or so.

Thanks,

	Ingo

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


#1612672

FromPeter Zijlstra <peterz@infradead.org>
Date2017-03-30 09:40 +0200
Message-ID<tqCQy-2Ql-27@gated-at.bofh.it>
In reply to#1612648
On Thu, Mar 30, 2017 at 09:17:07AM +0200, Ingo Molnar wrote:
> 
> * Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Wed, Mar 29, 2017 at 11:16:31PM +0200, Arnd Bergmann wrote:
> > > The latest change to the BUG() macro inadvertently reverted the earlier
> > > commit b06dd879f5db ("x86: always define BUG() and HAVE_ARCH_BUG, even
> > > with !CONFIG_BUG") that sanitized the behavior with CONFIG_BUG=n.
> > > 
> > > I noticed this as some warnings have appeared again that were previously
> > > fixed as a side effect of that patch:
> > > 
> > > kernel/seccomp.c: In function '__seccomp_filter':
> > > kernel/seccomp.c:670:1: error: no return statement in function returning non-void [-Werror=return-type]
> > > 
> > > drivers/gpu/drm/i915/intel_sprite.c: In function 'intel_check_sprite_plane':
> > > drivers/gpu/drm/i915/intel_sprite.c:936:20: error: 'src_h' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > >    src->y2 = (src_y + src_h) << 16;
> > >              ~~~~~~~^~~~~~~~
> > > drivers/gpu/drm/i915/intel_sprite.c:934:20: error: 'src_w' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > >    src->x2 = (src_x + src_w) << 16;
> > >              ~~~~~~~^~~~~~~~
> > > drivers/gpu/drm/i915/intel_sprite.c:936:20: error: 'src_y' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > >    src->y2 = (src_y + src_h) << 16;
> > >              ~~~~~~~^~~~~~~~
> > > drivers/gpu/drm/i915/intel_sprite.c:934:20: error: 'src_x' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > >    src->x2 = (src_x + src_w) << 16;
> > >              ~~~~~~~^~~~~~~~
> > > 
> > > This combines the two patches and uses the ud2 macro to define BUG()
> > > in case of CONFIG_BUG=n.
> > 
> > OK, fair enough I suppose. However, I cribbed this from arm64. What does
> > that do for BUG=n ?
> 
> I think we'll get a U2D crash in this case, without any bug information.
> 
> I.e. only marginally debuggable, but it's a deterministic outcome - instead of the 
> crazy GCC code generation variant of the day when the warning triggers, or the 
> similarly crazy infinite loop hang.
> 
> I'm not entirely sure though, I don't think many people actually _use_ 
> CONFIG_BUG=n, it's essentially a crazy thing to do even on constrainted hardware. 
> Debugging and maintenance costs almost always trump marginal hardware costs of a 
> bit more debugging code.

So should we then, for x86, disable BUG=n instead?

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


#1612676

FromIngo Molnar <mingo@kernel.org>
Date2017-03-30 09:50 +0200
Message-ID<tqD0e-2Ug-29@gated-at.bofh.it>
In reply to#1612672
* Peter Zijlstra <peterz@infradead.org> wrote:

> On Thu, Mar 30, 2017 at 09:17:07AM +0200, Ingo Molnar wrote:
> > 
> > * Peter Zijlstra <peterz@infradead.org> wrote:
> > 
> > > On Wed, Mar 29, 2017 at 11:16:31PM +0200, Arnd Bergmann wrote:
> > > > The latest change to the BUG() macro inadvertently reverted the earlier
> > > > commit b06dd879f5db ("x86: always define BUG() and HAVE_ARCH_BUG, even
> > > > with !CONFIG_BUG") that sanitized the behavior with CONFIG_BUG=n.
> > > > 
> > > > I noticed this as some warnings have appeared again that were previously
> > > > fixed as a side effect of that patch:
> > > > 
> > > > kernel/seccomp.c: In function '__seccomp_filter':
> > > > kernel/seccomp.c:670:1: error: no return statement in function returning non-void [-Werror=return-type]
> > > > 
> > > > drivers/gpu/drm/i915/intel_sprite.c: In function 'intel_check_sprite_plane':
> > > > drivers/gpu/drm/i915/intel_sprite.c:936:20: error: 'src_h' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > >    src->y2 = (src_y + src_h) << 16;
> > > >              ~~~~~~~^~~~~~~~
> > > > drivers/gpu/drm/i915/intel_sprite.c:934:20: error: 'src_w' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > >    src->x2 = (src_x + src_w) << 16;
> > > >              ~~~~~~~^~~~~~~~
> > > > drivers/gpu/drm/i915/intel_sprite.c:936:20: error: 'src_y' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > >    src->y2 = (src_y + src_h) << 16;
> > > >              ~~~~~~~^~~~~~~~
> > > > drivers/gpu/drm/i915/intel_sprite.c:934:20: error: 'src_x' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > > >    src->x2 = (src_x + src_w) << 16;
> > > >              ~~~~~~~^~~~~~~~
> > > > 
> > > > This combines the two patches and uses the ud2 macro to define BUG()
> > > > in case of CONFIG_BUG=n.
> > > 
> > > OK, fair enough I suppose. However, I cribbed this from arm64. What does
> > > that do for BUG=n ?
> > 
> > I think we'll get a U2D crash in this case, without any bug information.
> > 
> > I.e. only marginally debuggable, but it's a deterministic outcome - instead of the 
> > crazy GCC code generation variant of the day when the warning triggers, or the 
> > similarly crazy infinite loop hang.
> > 
> > I'm not entirely sure though, I don't think many people actually _use_ 
> > CONFIG_BUG=n, it's essentially a crazy thing to do even on constrainted hardware. 
> > Debugging and maintenance costs almost always trump marginal hardware costs of a 
> > bit more debugging code.
> 
> So should we then, for x86, disable BUG=n instead?

Arnd, does CONFIG_BUG=n give any (marginal) text savings wrt. 
CONFIG_BUG=y && CONFIG_BUG_VERBOSE=n?

If not then we should indeed just disable CONFIG_BUG=n.

Thanks,

	Ingo

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


#1612691

FromArnd Bergmann <arnd@arndb.de>
Date2017-03-30 10:10 +0200
Message-ID<tqDjA-3gZ-9@gated-at.bofh.it>
In reply to#1612676
On Thu, Mar 30, 2017 at 9:47 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Peter Zijlstra <peterz@infradead.org> wrote:
>> So should we then, for x86, disable BUG=n instead?
>
> Arnd, does CONFIG_BUG=n give any (marginal) text savings wrt.
> CONFIG_BUG=y && CONFIG_BUG_VERBOSE=n?

My patch makes it slightly worse, but the difference is still
around 1% of the total vmlinux size:

# CONFIG_BUG=n without my patch
   text   data    bss    dec    hex filename
9543165 4190288 843776 14577229 de6e4d obj-x86/vmlinux

# CONFIG_BUG=n with my patch
   text   data    bss    dec    hex filename
9578700 4190480 843776 14612956 def9dc obj-x86/vmlinux

# CONFIG_BUG=y, CONFIG_BUGVERBOSE=n
   text   data    bss    dec    hex filename
9698166 4212208 843776 14754150 e12166 obj-x86/vmlinux

Among the things we save are:

- WARN()/WARN_ON() gets left out entirely
- The bug_entry section (46192 bytes in defconfig)
- The code for decoding and printing the bug

> If not then we should indeed just disable CONFIG_BUG=n.

CONFIG_BUG is already guarded by CONFIG_EXPERT, along with
this help text in Kconfig:

          Disabling this option eliminates support for BUG and WARN, reducing
          the size of your kernel image and potentially quietly ignoring
          numerous fatal conditions. You should only consider disabling this
          option for embedded systems with no facilities for reporting errors.
          Just say Y.

I think this is reasonable to disable for systems that have no console
at all.

        Arnd

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


#1612657

FromArnd Bergmann <arnd@arndb.de>
Date2017-03-30 09:30 +0200
Message-ID<tqCGS-2MS-21@gated-at.bofh.it>
In reply to#1612642
On Thu, Mar 30, 2017 at 9:10 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, Mar 29, 2017 at 11:16:31PM +0200, Arnd Bergmann wrote:
>> The latest change to the BUG() macro inadvertently reverted the earlier
>> commit b06dd879f5db ("x86: always define BUG() and HAVE_ARCH_BUG, even
>> with !CONFIG_BUG") that sanitized the behavior with CONFIG_BUG=n.
>>
>> I noticed this as some warnings have appeared again that were previously
>> fixed as a side effect of that patch:
>>
>> kernel/seccomp.c: In function '__seccomp_filter':
>> kernel/seccomp.c:670:1: error: no return statement in function returning non-void [-Werror=return-type]
>>
>> drivers/gpu/drm/i915/intel_sprite.c: In function 'intel_check_sprite_plane':
>> drivers/gpu/drm/i915/intel_sprite.c:936:20: error: 'src_h' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>    src->y2 = (src_y + src_h) << 16;
>>              ~~~~~~~^~~~~~~~
>> drivers/gpu/drm/i915/intel_sprite.c:934:20: error: 'src_w' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>    src->x2 = (src_x + src_w) << 16;
>>              ~~~~~~~^~~~~~~~
>> drivers/gpu/drm/i915/intel_sprite.c:936:20: error: 'src_y' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>    src->y2 = (src_y + src_h) << 16;
>>              ~~~~~~~^~~~~~~~
>> drivers/gpu/drm/i915/intel_sprite.c:934:20: error: 'src_x' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>    src->x2 = (src_x + src_w) << 16;
>>              ~~~~~~~^~~~~~~~
>>
>> This combines the two patches and uses the ud2 macro to define BUG()
>> in case of CONFIG_BUG=n.
>
> OK, fair enough I suppose. However, I cribbed this from arm64.

Catalin just merged my patch to make arm64 work like x86 and arm32 ;-)

> What does that do for BUG=n ?

Without the patch, running into a BUG() often leads to undefined
behavior, e.g. running off at the end of a function that is marked
noreturn(), or simply continuing a thread after we know that this
leads to data corruption (which is the reason to put the BUG() in there
in the first place).

With my patch, we will insert a ud2 instruction and mark the context
as unreachable(), so gcc can discard any assembler output after the
unreachable() and the kernel will kill the task at that point, but will
not have a bug_entry and just report an invalid instruction similar
to what newer gcc generates for an unconditional division by zero or
NULL pointer dereference.

     Arnd

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


#1612668 — [tip:x86/asm] x86/debug: Define BUG() again for !CONFIG_BUG

Fromtip-bot for Arnd Bergmann <tipbot@zytor.com>
Date2017-03-30 09:40 +0200
Subject[tip:x86/asm] x86/debug: Define BUG() again for !CONFIG_BUG
Message-ID<tqCQy-2Ql-9@gated-at.bofh.it>
In reply to#1612353
Commit-ID:  70579a86e3c8eb2ce57999e594a73b4dfe095959
Gitweb:     http://git.kernel.org/tip/70579a86e3c8eb2ce57999e594a73b4dfe095959
Author:     Arnd Bergmann <arnd@arndb.de>
AuthorDate: Wed, 29 Mar 2017 23:16:31 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 30 Mar 2017 09:12:10 +0200

x86/debug: Define BUG() again for !CONFIG_BUG

The latest change to the BUG() macro inadvertently reverted the earlier
commit:

  b06dd879f5db ("x86: always define BUG() and HAVE_ARCH_BUG, even with !CONFIG_BUG")

... that sanitized the behavior with CONFIG_BUG=n.

I noticed this as some warnings have appeared again that were previously
fixed as a side effect of that patch:

  kernel/seccomp.c: In function '__seccomp_filter':
  kernel/seccomp.c:670:1: error: no return statement in function returning non-void [-Werror=return-type]
  ...

This combines the two patches and uses the ud2 macro to define BUG()
in case of CONFIG_BUG=n.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Fixes: 9a93848fe787 ("x86/debug: Implement __WARN() using UD0")
Link: http://lkml.kernel.org/r/20170329211646.2707365-1-arnd@arndb.de
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/bug.h | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index 4fde330..cecf559 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -23,7 +23,6 @@
 #define LEN_UD0		2
 
 #ifdef CONFIG_GENERIC_BUG
-#define HAVE_ARCH_BUG
 
 #ifdef CONFIG_X86_32
 # define __BUG_REL(val)	".long " __stringify(val)
@@ -64,6 +63,13 @@ do {									\
 
 #endif /* CONFIG_DEBUG_BUGVERBOSE */
 
+#else
+
+#define _BUG_FLAGS(ins, flags)  asm volatile(ins)
+
+#endif /* CONFIG_GENERIC_BUG */
+
+#define HAVE_ARCH_BUG
 #define BUG()							\
 do {								\
 	_BUG_FLAGS(ASM_UD2, 0);					\
@@ -72,8 +78,6 @@ do {								\
 
 #define __WARN_TAINT(taint)	_BUG_FLAGS(ASM_UD0, BUGFLAG_TAINT(taint))
 
-#endif /* CONFIG_GENERIC_BUG */
-
 #include <asm-generic/bug.h>
 
 #endif /* _ASM_X86_BUG_H */

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web