public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: David Woodhouse <dwmw2@infradead.org>,
	Josh Poimboeuf <jpoimboe@redhat.com>
Cc: linux-kernel@vger.kernel.org, Dave Hansen <dave.hansen@intel.com>,
	Ashok Raj <ashok.raj@intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Tim Chen <tim.c.chen@linux.intel.com>,
	Andy Lutomirski <luto@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Greg KH <gregkh@linuxfoundation.org>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Andi Kleen <ak@linux.intel.com>,
	Arjan Van De Ven <arjan.van.de.ven@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Jun Nakajima <jun.nakajima@intel.com>,
	Asit Mallick <asit.k.mallick@intel.com>,
	Peter Zijlstra <peterz@infradead.org>
Subject: [PATCH v2 10/10] objtool: More complex static jump implementation
Date: Tue, 16 Jan 2018 15:28:35 +0100	[thread overview]
Message-ID: <20180116143241.478561014@infradead.org> (raw)
In-Reply-To: 20180116142825.376986833@infradead.org

[-- Attachment #1: peterz-objtool-fancy.patch --]
[-- Type: text/plain, Size: 3272 bytes --]

When using something like:

  -#define sched_feat(x) (static_branch_##x(&sched_feat_keys[__SCHED_FEAT_##x]))
  +#define sched_feat(x) (static_branch_##x(&sched_feat_keys[__SCHED_FEAT_##x]) && \
  +			(arch_static_assert(), true))

we get an objtool assertion fail like:

kernel/sched/fair.o: warning: objtool: hrtick_update()+0xd: static assert FAIL

where:

0000000000001140 <hrtick_update>:
    1140:       0f 1f 44 00 00          nopl   0x0(%rax,%rax,1)
    1145:       c3                      retq
    1146:       48 8b b7 30 09 00 00    mov    0x930(%rdi),%rsi
    114d:       8b 87 d8 09 00 00       mov    0x9d8(%rdi),%eax
    1153:       48 0f a3 05 00 00 00    bt     %rax,0x0(%rip)        # 115b <hrtick_update+0x1b>
    115a:       00
                        1157: R_X86_64_PC32     __cpu_active_mask-0x4

and:

RELOCATION RECORDS FOR [__jump_table]:
0000000000000150 R_X86_64_64       .text+0x0000000000001140
0000000000000158 R_X86_64_64       .text+0x0000000000001146

RELOCATION RECORDS FOR [.discard.jump_assert]:
0000000000000028 R_X86_64_64       .text+0x000000000000114d

IOW, GCC managed to place the assertion 1 instruction _after_ the
static jump target.

So while the code generation is fine, the assertion gets placed wrong.
We can 'fix' this by not only considering the immediate static jump
locations but also all the unconditional code after it, terminating
the basic block on any unconditional instruction or branch entry
point.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/check.c |   40 ++++++++++++++++++++++++++++++++++++++++
 tools/objtool/check.h |    2 +-
 2 files changed, 41 insertions(+), 1 deletion(-)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -520,6 +520,8 @@ static int add_jump_destinations(struct
 				  dest_off);
 			return -1;
 		}
+
+		insn->jump_dest->branch_target = true;
 	}
 
 	return 0;
@@ -1197,6 +1199,40 @@ static int read_retpoline_hints(struct o
 	return 0;
 }
 
+static int grow_static_blocks(struct objtool_file *file)
+{
+	struct instruction *insn;
+	bool static_block = false;
+
+	for_each_insn(file, insn) {
+		if (!static_block && !insn->static_jump_dest)
+			continue;
+
+		if (insn->static_jump_dest) {
+			static_block = true;
+			continue;
+		}
+
+		if (insn->branch_target) {
+			static_block = false;
+			continue;
+		} else switch (insn->type) {
+		case INSN_JUMP_CONDITIONAL:
+		case INSN_JUMP_DYNAMIC:
+		case INSN_CALL:
+		case INSN_CALL_DYNAMIC:
+		case INSN_RETURN:
+		case INSN_BUG:
+			static_block = false;
+			continue;
+		}
+
+		insn->static_jump_dest = static_block;
+	}
+
+	return 0;
+}
+
 static int decode_sections(struct objtool_file *file)
 {
 	int ret;
@@ -1239,6 +1275,10 @@ static int decode_sections(struct objtoo
 	if (ret)
 		return ret;
 
+	ret = grow_static_blocks(file);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -45,7 +45,7 @@ struct instruction {
 	unsigned char type;
 	unsigned long immediate;
 	bool alt_group, visited, dead_end, ignore, hint, save, restore, ignore_alts;
-	bool static_jump_dest, retpoline_safe;
+	bool static_jump_dest, retpoline_safe, branch_target;
 	struct symbol *call_dest;
 	struct instruction *jump_dest;
 	struct list_head alts;

  parent reply	other threads:[~2018-01-16 14:34 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-16 14:28 [PATCH v2 00/10] objtool validation of static branches and retpoline Peter Zijlstra
2018-01-16 14:28 ` [PATCH v2 01/10] x86: Reindent _static_cpu_has Peter Zijlstra
2018-01-16 15:48   ` Borislav Petkov
2018-01-16 14:28 ` [PATCH v2 02/10] x86: Update _static_cpu_has to use all named variables Peter Zijlstra
2018-01-18 11:21   ` Borislav Petkov
2018-01-18 15:09     ` Peter Zijlstra
2018-01-18 15:24       ` Borislav Petkov
2018-01-16 14:28 ` [PATCH v2 03/10] x86: Add a type field to alt_instr Peter Zijlstra
2018-01-16 22:49   ` Josh Poimboeuf
2018-01-16 22:53     ` Borislav Petkov
2018-01-16 23:06       ` Josh Poimboeuf
2018-01-18 11:32   ` Borislav Petkov
2018-01-16 14:28 ` [PATCH v2 04/10] objtool: Implement base jump_assert support Peter Zijlstra
2018-01-16 14:28 ` [PATCH v2 05/10] x86: Annotate static_cpu_has alternative Peter Zijlstra
2018-01-18 13:15   ` Borislav Petkov
2018-01-16 14:28 ` [PATCH v2 06/10] objtool: Implement jump_assert for _static_cpu_has() Peter Zijlstra
2018-01-16 23:02   ` Josh Poimboeuf
2018-01-17  9:19     ` Peter Zijlstra
2018-01-17 14:27       ` Josh Poimboeuf
2018-01-17 14:30         ` Josh Poimboeuf
2018-01-17 16:30           ` Peter Zijlstra
2018-01-16 14:28 ` [PATCH v2 07/10] x86/jump_label: Implement arch_static_assert() Peter Zijlstra
2018-01-18 13:33   ` Borislav Petkov
2018-01-18 15:31     ` Peter Zijlstra
2018-01-16 14:28 ` [PATCH v2 08/10] objtool: Add retpoline validation Peter Zijlstra
2018-01-16 14:28 ` [PATCH v2 09/10] x86: Annotate dynamic jump in head_64.S Peter Zijlstra
2018-01-16 14:28 ` Peter Zijlstra [this message]
2018-01-16 15:20   ` [PATCH v2 10/10] objtool: More complex static jump implementation Peter Zijlstra
2018-01-17  3:05   ` Josh Poimboeuf
2018-01-17  8:18     ` Peter Zijlstra
2018-01-16 19:49 ` [PATCH v2 11/10] objtool: Even more complex static block checks Peter Zijlstra
2018-01-17  3:12   ` Josh Poimboeuf
2018-01-17  8:13     ` Peter Zijlstra
2018-01-17 14:13       ` Josh Poimboeuf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180116143241.478561014@infradead.org \
    --to=peterz@infradead.org \
    --cc=aarcange@redhat.com \
    --cc=ak@linux.intel.com \
    --cc=arjan.van.de.ven@intel.com \
    --cc=ashok.raj@intel.com \
    --cc=asit.k.mallick@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave.hansen@intel.com \
    --cc=dwmw2@infradead.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jpoimboe@redhat.com \
    --cc=jun.nakajima@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=tim.c.chen@linux.intel.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox