linux-kernel - Re: Definition of BUG on x86

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-next>] [day] [month] [year] [list]

Message-Id: <1235043648.15053.35.camel@nathan.suse.cz>
Date:	Thu, 19 Feb 2009 12:40:48 +0100
From:	Petr Tesarik <ptesarik@...e.cz>
To:	Jeremy Fitzhardinge <jeremy@...p.org>
Cc:	LKML <linux-kernel@...r.kernel.org>
Subject: Re: Definition of BUG on x86

Jeremy Fitzhardinge píše v St 18. 02. 2009 v 09:38 -0800:
> Petr Tesarik wrote:
> > Hi Jeremy,
> >
> > while looking at the disassembly of a recent kernel, I noticed that
> > BUG() without CONFIG_DEBUG_BUGVERBOSE now produces a sequence like this:
> >
> > 0xc02c2f11 <schedule+581>:      ud2a
> > 0xc02c2f13 <schedule+583>:      jmp    0xc02c2f13 <schedule+583>
> >
> > I dug back into the git log and found out that this was last changed by
> > you in commit 91768d6c2bad0d2766a166f13f2f57e197de3458.
> >
> > I might be missing something, but why do we need that extra endless loop
> > there? Isn't ud2a simply a no-return? Especially given that there is no
> > such for(;;) for the CONFIG_DEBUG_BUGVERBOSE case.
> >
> > Now, it's no big deal, but it seems to me that the second instruction is
> > totally useless and merely occupies 2 bytes in the i-cache...
> >
> > Why not get rid of it like this?
> >   
> 
> The trouble is that the compiler doesn't know that the asm never 
> returns, and so it starts to complain about things that might happen 
> after the BUG returns.  In other words, the loop is just there to shut 
> the compiler up.  I agree that its unfortunate that it generates a real 
> instruction, but I couldn't find a better alternative.

Ah, yes, you're right. If you want to make gcc aware of the fact that a
function never returns, it must recognize that there is no path to that
function's EXIT, otherwise it will emit a warning (which is pretty
annoying, indeed). Since we don't want to make a call/jmp to an external
noreturn function, so the no-return code must be directly visible to
gcc. Unfortunately, there is no way of telling gcc that an asm statement
does not return.

Actually, it would be a good thing if you could attach a special
__attribute__ to the asm to mark it that way. But it's not there, so gcc
_must_ generate the code itself. :|

I thought about putting the endless loop into a separate section which
would get discarded by the linker. But that doesn't work either, because
we need a .pushsection _and_ a .popsection afterwards, but there is no
way of emitting that .popsection, because gcc discards everything and
anything after the endless loop.

To recap:

 1. gcc must emit the code for the endless loop
 2. we do not want to have the code in the resulting binary
 3. gcc will not let us insert anything after the loop

So, the only method I could invent was using gas macros. It works but is
quite ugly, because it relies on the actual assembler instruction which
is generated by the compiler. Now, AFAIK gcc has always translated
"for(;;)" into a jump to self, and that with any conceivable compiler
options, but I don't know anything about Intel cc.

Anyway, if you think the ugliness is acceptable, you may take my patch.

Signed-off-by: Petr Tesarik <ptesarik@...e.cz>

diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index d9cf1cd..004093a 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -4,6 +4,14 @@
 #ifdef CONFIG_BUG
 #define HAVE_ARCH_BUG

+static inline __noreturn void discarded_jmp(void)
+{
+	asm volatile(".macro jmp target\n"
+		     "\t.purgem jmp\n"
+		     ".endm\n");
+	for (;;) ;
+}
+
 #ifdef CONFIG_DEBUG_BUGVERBOSE

 #ifdef CONFIG_X86_32
@@ -22,14 +30,14 @@ do {								\
 		     ".popsection"				\
 		     : : "i" (__FILE__), "i" (__LINE__),	\
 		     "i" (sizeof(struct bug_entry)));		\
-	for (;;) ;						\
+	discarded_jmp();					\
 } while (0)

 #else
 #define BUG()							\
 do {								\
 	asm volatile("ud2");					\
-	for (;;) ;						\
+	discarded_jmp();					\
 } while (0)
 #endif

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