linux-kernel - Re: [PATCH 4/4] bug: mark disabled BUG() as unreachable() code

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]

Date:	Fri, 27 Apr 2012 22:21:04 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Konstantin Khlebnikov <khlebnikov@...nvz.org>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 4/4] bug: mark disabled BUG() as unreachable() code

On Fri, Apr 27, 2012 at 10:10 PM, Konstantin Khlebnikov
<khlebnikov@...nvz.org> wrote:
>
> With this patch gcc throw out loop at the end of do_exit():

I think that's the least of our problems - that loop only exists to
get rid of a warning, and the return from that schedule() really is
supposed to be unreachable().

What makes me much more worried is that iirc, gcc will use
"unreachable()" to also get rid of function epilogues etc, which is
perfectly fine if it really is something entirely unreachable. But the
kernel use of BUG() is often as a kind of "assert()", and if gcc
generates actively bad code for it (and it does, with unreachable()),
it turns the turned-off BUG() into something *really* horrible that
makes for a debugging disaster.

So some pattern like

   if (badness)
      BUG();

would generate code that is actively insane, instead of (like now)
generating basically a no-op.

And THAT makes me go "Eww". "unreachable()" should only be used in
situations that really are very very unreachable() (eg after an "asm"
that goes off to la-la-land or a call to "exit()" in user land etc).

The kernel kinds of "assert" is *hopefully* not reachable, but if we
ever reach it, we don't want to make things worse. The BUG() was there
to give us a nicer debug message about what went wrong, and I think
your patch actively destroys that whole thing and makes for something
*worse*.

                    Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/