linux-kernel - Re: [PATCH v2 11/10] objtool: Even more complex static block checks

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: <20180117081309.GJ2228@hirez.programming.kicks-ass.net>
Date:   Wed, 17 Jan 2018 09:13:09 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     David Woodhouse <dwmw2@...radead.org>,
        linux-kernel@...r.kernel.org, Dave Hansen <dave.hansen@...el.com>,
        Ashok Raj <ashok.raj@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        Andy Lutomirski <luto@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Arjan Van De Ven <arjan.van.de.ven@...el.com>,
        Dan Williams <dan.j.williams@...el.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Jun Nakajima <jun.nakajima@...el.com>,
        Asit Mallick <asit.k.mallick@...el.com>
Subject: Re: [PATCH v2 11/10] objtool: Even more complex static block checks

On Tue, Jan 16, 2018 at 09:12:32PM -0600, Josh Poimboeuf wrote:
> On Tue, Jan 16, 2018 at 08:49:17PM +0100, Peter Zijlstra wrote:
> > Subject: objtool: Even more complex static block checks
> > From: Peter Zijlstra <peterz@...radead.org>
> > Date: Tue Jan 16 20:17:01 CET 2018
> > 
> > I've observed GCC transform:
> > 
> >   f()
> >   {
> > 	  if (!static_branch_unlikely())
> > 		  return;
> > 
> > 	  static_assert();
> > 	  A;
> >   }
> > 
> >   g()
> >   {
> > 	  f();
> >   }
> > 
> > Into:
> > 
> >   f()
> >   {
> > 	  static_assert();
> > 	  A;
> >   }
> > 
> >   g()
> >   {
> > 	  if (static_branch_unlikely())
> > 		  f();
> >   }
> > 
> > Which results in the assertion landing at f+0. The transformation is
> > valid and useful; it avoids a pointless CALL+RET sequence, so we'll
> > have to teach objtool how to deal with this.
> > 
> > Do this by marking all CALL destinations with static_call when called
> > from a static_block and non_static_call when called outside a
> > static_block. This allows us to identify functions called exclusively
> > from a static_block and start them with a static_block.
> 
> Ew... where'd you place the assertion to trigger this?

Its the patch I pastebin'ed you earlier, also see below.

> It's late and my brain has already clocked out, so I'll need to revisit
> this tomorrow.  But now I'm wondering if my basic block idea would be a
> better way to solve this.

I would think basic-blocks are inside functions, and this patch goes
across functions, something you'd still need even if you had basic
blocks.

Also, basic blocks are non-trivial because they can overlap. I've
implemented something like that before for perf, see commit:

  70fbe0574558 ("perf annotate: Add branch stack / basic block")

We could probably lift that code fairly easily.

---
Subject: jump_label: Add static assertion to every static_branch
From: Peter Zijlstra <peterz@...radead.org>
Date: Tue Jan 16 15:27:36 CET 2018

for testing.. not sure we wants this in general

Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
---
 arch/x86/include/asm/jump_label.h |    1 +
 include/linux/jump_label.h        |    8 ++++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

--- a/arch/x86/include/asm/jump_label.h
+++ b/arch/x86/include/asm/jump_label.h
@@ -76,6 +76,7 @@ static __always_inline bool arch_static_
  *
  * Also works with static_cpu_has().
  */
+#define arch_static_assert arch_static_assert
 static __always_inline void arch_static_assert(void)
 {
 	asm volatile ("1:\n\t"
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -323,6 +323,10 @@ extern bool ____wrong_branch_error(void)
 
 #ifdef HAVE_JUMP_LABEL
 
+#ifndef arch_static_assert
+#define arch_static_assert (void)
+#endif
+
 /*
  * Combine the right initial value (type) with the right branch order
  * to generate the desired result.
@@ -388,7 +392,7 @@ extern bool ____wrong_branch_error(void)
 		branch = !arch_static_branch_jump(&(x)->key, true);		\
 	else									\
 		branch = ____wrong_branch_error();				\
-	branch;									\
+	branch && (arch_static_assert(), true);					\
 })
 
 #define static_branch_unlikely(x)						\
@@ -400,7 +404,7 @@ extern bool ____wrong_branch_error(void)
 		branch = arch_static_branch(&(x)->key, false);			\
 	else									\
 		branch = ____wrong_branch_error();				\
-	branch;									\
+	branch && (arch_static_assert(), true);					\
 })
 
 #else /* !HAVE_JUMP_LABEL */
> Josh