linux-kernel - Re: [PATCH] x86/debug: define BUG() againfor !CONFIG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK8P3a2s0ic4s-kJGxjnGr1883TX1MtZfcWX=-ibiu4Yu3tT9A@mail.gmail.com>
Date:   Thu, 30 Mar 2017 09:25:19 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Ingo Molnar <mingo@...hat.com>,
        Josh Triplett <josh@...htriplett.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H. Peter Anvin" <hpa@...or.com>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] x86/debug: define BUG() againfor !CONFIG_BUG

On Thu, Mar 30, 2017 at 9:10 AM, Peter Zijlstra <peterz@...radead.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