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

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] [day] [month] [year] [list]

Message-ID: <20200131160518.ou6zadyo5qvkr2xd@treble>
Date:   Fri, 31 Jan 2020 10:05:18 -0600
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Borislav Petkov <bp@...e.de>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        "Luck, Tony" <tony.luck@...el.com>, Ingo Molnar <mingo@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Borislav Petkov <bp@...en8.de>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Julien Thierry <jthierry@...hat.com>
Subject: Re: [GIT PULL] x86/asm changes for v5.6

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;
 }