Re: [GIT PULL] x86/asm changes for v5.6

From: Josh Poimboeuf
Date: Fri Jan 31 2020 - 11:05:28 EST


On Fri, Jan 31, 2020 at 03:27:27PM +0100, Borislav Petkov wrote:
> + Josh.
>
> On Thu, Jan 30, 2020 at 10:02:54AM -0800, Linus Torvalds wrote:
> > Maybe with the exception that a short conditional jump inside the
> > alternative code itself is fine.
> >
> > Because a branch-over inside the alternative sequence (or a loop -
> > think inline cmpxchg loop or whatever) would be fine, since it's
> > unaffected by code placement.
>
> Perhaps something like the below as a start.
>
> The build becomes really noisy if the error case is hit because aborting
> in handle_group_alt() really messes up objtool processing but that is
> perhaps ok as the idea is to *see* that something's wrong.

[ Adding Julien as an FYI -- not sure how it affects arm64 ]

Boris, I made the check even broader than we discussed on IRC: it
disallows *any* relocations in the alternatives section unless it's the
first instruction in the group and it's a call or a jmp.

Untested, let me know if it works.

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index b6da413bcbd6..382a65363379 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -782,6 +782,20 @@ static int handle_group_alt(struct objtool_file *file,
insn->ignore = orig_insn->ignore_alts;
insn->func = orig_insn->func;

+ /*
+ * The x86 alternatives code adjusts relocation targets only in
+ * the case where the first instruction is either a call or a
+ * jump. Otherwise, relocations aren't supported by the
+ * alternatives code (as there hasn't yet been a need for it).
+ */
+ if (!(insn->offset == special_alt->new_off &&
+ (insn->type == INSN_CALL || insn->type == INSN_JUMP_UNCONDITIONAL)) &&
+ find_rela_by_dest_range(insn->sec, insn->offset, insn->len)) {
+ WARN_FUNC("relocations not allowed in alternatives section",
+ insn->sec, insn->offset);
+ return -1;
+ }
+
if (insn->type != INSN_JUMP_CONDITIONAL &&
insn->type != INSN_JUMP_UNCONDITIONAL)
continue;
@@ -2509,8 +2523,14 @@ int check(const char *_objname, bool orc)
out:
cleanup(&file);

- /* ignore warnings for now until we get all the code cleaned up */
- if (ret || warnings)
- return 0;
+ if (ret < 0) {
+ /*
+ * Fatal error. The binary is corrupt or otherwise broken in
+ * some way, or objtool is badly broken. Fail the kernel
+ * build.
+ */
+ return ret;
+ }
+
return 0;
}