linux-kernel - Re: [PATCH v4 02/10] objtool: Handle special cases of dead end insn

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: <20241126143243.GN38837@noisy.programming.kicks-ass.net>
Date: Tue, 26 Nov 2024 15:32:43 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: Tiezhu Yang <yangtiezhu@...ngson.cn>
Cc: Josh Poimboeuf <jpoimboe@...nel.org>,
	Huacai Chen <chenhuacai@...nel.org>, loongarch@...ts.linux.dev,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 02/10] objtool: Handle special cases of dead end insn

On Tue, Nov 26, 2024 at 06:42:15PM +0800, Tiezhu Yang wrote:
> On 11/26/2024 02:45 PM, Josh Poimboeuf wrote:
> > On Fri, Nov 22, 2024 at 12:49:57PM +0800, Tiezhu Yang wrote:
> > > There are some "unreachable instruction" objtool warnings when compling
> > > with Clang on LoongArch, this is because the "break" instruction is set
> > > as dead end due to its type is INSN_BUG in decode_instructions() at the
> > > beginning, and it does not set insn->dead_end of the "break" instruction
> > > as false after checking ".rela.discard.reachable" in add_dead_ends(), so
> > > the next instruction of "break" is marked as unreachable.
> > > 
> > > Actually, it can find the reachable instruction after parsing the section
> > > ".rela.discard.reachable", in some cases, the "break" instruction may not
> > > be the first previous instruction with scheduling by Machine Instruction
> > > Scheduler of LLVM, it should find more times and then set insn->dead_end
> > > of the "break" instruction as false.
> > > 
> > > This is preparation for later patch on LoongArch, there is no effect for
> > > the other archs with this patch.
> > > 
> > > Signed-off-by: Tiezhu Yang <yangtiezhu@...ngson.cn>
> > 
> > I'm having trouble understanding this commit log, is the problem that
> > the compiler is sometimes inserting code between 'break' and the
> > unreachable() inline asm?
> > 
> > If so, this sounds like a problem that was already solved for x86 with:
> > 
> >   bfb1a7c91fb7 ("x86/bug: Merge annotate_reachable() into _BUG_FLAGS() asm")
> > 
> > Can you check if that fixes it?
> 
> I will try, thank you.
> 

I was poking at the reachable annotations and ended up with this:

--- a/arch/loongarch/include/asm/bug.h
+++ b/arch/loongarch/include/asm/bug.h
@@ -4,6 +4,7 @@
 
 #include <asm/break.h>
 #include <linux/stringify.h>
+#include <linux/objtool.h>
 
 #ifndef CONFIG_DEBUG_BUGVERBOSE
 #define _BUGVERBOSE_LOCATION(file, line)
@@ -37,21 +38,21 @@
 
 #define ASM_BUG()	ASM_BUG_FLAGS(0)
 
-#define __BUG_FLAGS(flags)					\
-	asm_inline volatile (__stringify(ASM_BUG_FLAGS(flags)));
+#define __BUG_FLAGS(flags, extra)					\
+	asm_inline volatile (__stringify(ASM_BUG_FLAGS(flags))		\
+			     extra);
 
 #define __WARN_FLAGS(flags)					\
 do {								\
 	instrumentation_begin();				\
-	__BUG_FLAGS(BUGFLAG_WARNING|(flags));			\
-	annotate_reachable();					\
+	__BUG_FLAGS(BUGFLAG_WARNING|(flags), ASM_REACHABLE);	\
 	instrumentation_end();					\
 } while (0)
 
 #define BUG()							\
 do {								\
 	instrumentation_begin();				\
-	__BUG_FLAGS(0);						\
+	__BUG_FLAGS(0, "");					\
 	unreachable();						\
 } while (0)