From: Peter Zijlstra <peterz@infradead.org>
To: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Jann Horn <jannh@google.com>, Andy Lutomirski <luto@kernel.org>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
"H. Peter Anvin" <hpa@zytor.com>,
the arch/x86 maintainers <x86@kernel.org>,
kernel list <linux-kernel@vger.kernel.org>,
alexandre.chartre@oracle.com
Subject: Re: x86 entry perf unwinding failure (missing IRET_REGS annotation on stack switch?)
Date: Tue, 28 Apr 2020 16:16:14 +0200 [thread overview]
Message-ID: <20200428141614.GA13616@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20200428124627.GC13558@hirez.programming.kicks-ass.net>
On Tue, Apr 28, 2020 at 02:46:27PM +0200, Peter Zijlstra wrote:
> On Tue, Apr 28, 2020 at 02:04:50AM -0500, Josh Poimboeuf wrote:
> > I'm thinking something like this should fix it. Peter, does this look
> > ok?
>
> Unfortunate. But also, I fear, insufficient. Specifically consider
> things like:
>
> ALTERNATIVE "jmp 1f",
> "alt...
> "..."
> "...insn", X86_FEAT_foo
> 1:
>
> This results in something like:
>
>
> .text .altinstr_replacement
> e8 xx ...
> 90
> 90
> ...
> 90
>
> Where all our normal single byte nops (0x90) are unreachable with
> undefined CFI, but the alternative might have CFI, which is never
> propagated.
>
> We ran into this with the validate_alternative stuff from Alexandre.
> So rather than hacking around this issue, should we not make
> create_orc() smarter?
>
> I'm trying to come up with something, but so far I'm just making a mess.
Like this, it's horrid, but it seems to work.
What do you think of the approach? I'll work on cleaning it up if you
don't hate it too much ;-)
---
diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c
index 9d2bf2daaaa6..2a853ae994ea 100644
--- a/tools/objtool/orc_gen.c
+++ b/tools/objtool/orc_gen.c
@@ -10,17 +10,129 @@
#include "check.h"
#include "warn.h"
-int create_orc(struct objtool_file *file)
+static bool same_cfi(struct cfi_state *a, struct cfi_state *b)
{
+ return memcmp(a, b, sizeof(*a));
+}
+
+static struct instruction *next_insn_same_sec(struct objtool_file *file,
+ struct instruction *insn)
+{
+ struct instruction *next = list_next_entry(insn, list);
+
+ if (!next || &next->list == &file->insn_list || next->sec != insn->sec)
+ return NULL;
+
+ return next;
+}
+
+struct alternative {
+ struct list_head list;
struct instruction *insn;
+ bool skip_orig;
+};
+
+static int alt_cfi(struct objtool_file *file,
+ struct instruction *prev_insn,
+ struct instruction *orig_insn,
+ struct instruction *alt_insn)
+{
+ unsigned long orig_offset = orig_insn->offset;
+ unsigned long alt_offset = alt_insn->offset;
+ struct instruction *next_insn;
+ bool orig_orc, alt_orc;
+
+ orig_orc = !prev_insn || same_cfi(&orig_insn->cfi, &prev_insn->cfi);
+ alt_orc = !prev_insn || same_cfi(&alt_insn->cfi, &prev_insn->cfi);
+
+again:
+ if ((orig_orc || alt_orc)) {
+ if (orig_insn->offset - orig_offset != alt_insn->offset - alt_offset) {
+ WARN_FUNC("alternative has unaligned ORC", orig_insn->sec, orig_insn->offset);
+ return -1;
+ }
+
+ if (orig_insn->visited) {
+ if (same_cfi(&orig_insn->cfi, &alt_insn->cfi)) {
+ WARN_FUNC("alternative violates ORC invariance", orig_insn->sec, orig_insn->offset);
+ return -1;
+ }
+ } else {
+ /*
+ * We're in unreachable NOPs, allow the alternative to
+ * override the CFI/ORC data.
+ */
+ orig_insn->cfi = alt_insn->cfi;
+ }
+ }
+
+ next_insn = next_insn_same_sec(file, alt_insn);
+ if (!next_insn)
+ return 0;
+
+ if (next_insn->offset == -1 /*FAKE_JUMP_OFFSET*/)
+ return 0;
+
+ alt_orc = same_cfi(&alt_insn->cfi, &next_insn->cfi);
+ alt_insn = next_insn;
+
+ do {
+ next_insn = next_insn_same_sec(file, orig_insn);
+ if (!next_insn)
+ return 0;
+ if (!next_insn->alt_group)
+ return 0;
+
+ orig_orc = next_insn->visited && same_cfi(&orig_insn->cfi, &next_insn->cfi);
+ orig_insn = next_insn;
+ } while (orig_insn->offset - orig_offset < alt_insn->offset - alt_offset);
+
+ goto again;
+}
+
+static void orig_fill(struct objtool_file *file,
+ struct instruction *prev_insn,
+ struct instruction *insn)
+{
+ for (;;) {
+ if (!insn->visited)
+ insn->cfi = prev_insn->cfi;
+
+ prev_insn = insn;
+ insn = next_insn_same_sec(file, insn);
+ if (!insn)
+ return;
+ if (!insn->alt_group)
+ return;
+ }
+}
+
+int create_orc(struct objtool_file *file)
+{
+ struct instruction *insn, *prev_insn = NULL;
for_each_insn(file, insn) {
struct orc_entry *orc = &insn->orc;
struct cfi_reg *cfa = &insn->cfi.cfa;
struct cfi_reg *bp = &insn->cfi.regs[CFI_BP];
+ int ret;
orc->end = insn->cfi.end;
+ if (insn->alt_group && !insn->ignore_alts) {
+ struct alternative *alt;
+
+ list_for_each_entry(alt, &insn->alts, list) {
+ if (alt->insn->offset == -1 /*FAKE_JUMP_OFFSET*/)
+ continue;
+ ret = alt_cfi(file, prev_insn, insn, alt->insn);
+ if (ret)
+ return ret;
+ }
+
+ orig_fill(file, prev_insn, insn);
+ }
+
if (cfa->base == CFI_UNDEFINED) {
orc->sp_reg = ORC_REG_UNDEFINED;
continue;
@@ -76,6 +188,8 @@ int create_orc(struct objtool_file *file)
orc->sp_offset = cfa->offset;
orc->bp_offset = bp->offset;
orc->type = insn->cfi.type;
+
+ prev_insn = insn;
}
return 0;
next prev parent reply other threads:[~2020-04-28 14:16 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-01 6:02 x86 entry perf unwinding failure (missing IRET_REGS annotation on stack switch?) Jann Horn
2020-03-02 14:58 ` Peter Zijlstra
2020-03-02 15:18 ` Josh Poimboeuf
2020-03-02 15:52 ` Josh Poimboeuf
2020-04-28 7:04 ` Josh Poimboeuf
2020-04-28 12:46 ` Peter Zijlstra
2020-04-28 14:14 ` Josh Poimboeuf
2020-04-28 14:35 ` Peter Zijlstra
2020-04-28 14:16 ` Peter Zijlstra [this message]
2020-04-28 14:31 ` Josh Poimboeuf
2020-04-28 15:25 ` Peter Zijlstra
2020-04-28 15:49 ` Josh Poimboeuf
2020-04-28 15:54 ` Josh Poimboeuf
2020-04-28 16:25 ` Sean Christopherson
2020-04-28 16:33 ` Josh Poimboeuf
2020-04-28 18:28 ` Peter Zijlstra
2020-04-28 16:44 ` Peter Zijlstra
2020-04-28 17:01 ` 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=20200428141614.GA13616@hirez.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=alexandre.chartre@oracle.com \
--cc=bp@alien8.de \
--cc=hpa@zytor.com \
--cc=jannh@google.com \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.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;
as well as URLs for NNTP newsgroup(s).