linux-kernel - Re: [PATCH RESEND] bug: When !CONFIG_BUG, simplify WARN_ON

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: <1747419.6FxgzfPen0@wuerfel>
Date:	Mon, 24 Feb 2014 13:42:09 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	David Howells <dhowells@...hat.com>
Cc:	Josh Triplett <josh@...htriplett.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-arch@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family

On Monday 24 February 2014 12:09:11 David Howells wrote:
> Josh Triplett <josh@...htriplett.org> wrote:
> 
> > > This means we actually want BUG() to end with __builtin_unreachable()
> > > as in the CONFIG_BUG=y case, and also ensure it actually is
> > > unreachable. As I have shown in [1], the there is a small overhead
> > > of doing this in terms of code size.
> > 
> > I agree that allowing BUG() to become a no-op seems suboptimal, if only
> > because of the resulting warnings and mis-optimizations.  However, I
> > think the overhead could be cut down massively, such that BUG() just
> > compiles down to a one-byte undefined instruction.  (__builtin_trap()
> > might do the right thing here; worth checking.)
> 
> Is it possible to use an inline function with an empty body in this?  Let the
> compiler prune away all the arguments?
> 

I think you are thinking of a different problem. BUG() already takes no
arguments, but we get warnings for code like this:

int f(void)
{
	switch (global) {
	case 0:
		return 0;
	case 1:
		return do_something();
	default:
		BUG();
	}
}

BUG() normally causes a fault and we print helpful messages before killing
the task, and gcc knows we never continue because of the
__builtin_unreachable() annotation.

If BUG() is defined as 'do { } while (0)' in the example above, we get
a warning because the function may end without returning a number.
If we define it to 'do { unreachable(); } while (0)', we don't get a
warning, but we can get undefined behavior in the case we ever get to
the end of the function.

__builtin_trap() would be nice as an architecture independent method,
but I just checked what it does on ARM, and it just calls abort(),
which is probably not what we want in the kernel, especially as long
as the ARM abort() function calls BUG(), and most architectures don't
define it at all.

'do { } while (1)' is probably a reasonably generic way to not return
and should compile into a single instruction on most architectures.
panic() would be another option to do something that won't cause
additional data corruption, but it's a varargs function, so that won't
help you much when you just try to save object code size.

	Arnd
--
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/