linux-kernel - [RFC PATCH] x86/retpolines: Prevent speculation after RET

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: <20210218184639.GF4214@zn.tnic>
Date:   Thu, 18 Feb 2021 19:46:39 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     x86@...nel.org, tony.luck@...el.com, pjt@...gle.com,
        linux-kernel@...r.kernel.org, r.marek@...embler.cz,
        jpoimboe@...hat.com, jikos@...nel.org,
        Dave Hansen <dave.hansen@...el.com>,
        Andrew Cooper <andrew.cooper3@...rix.com>
Subject: [RFC PATCH] x86/retpolines: Prevent speculation after RET

On Thu, Feb 18, 2021 at 05:59:38PM +0100, Peter Zijlstra wrote:
> Hi!
> 
> The first patch rearranges the implementation and consolidates unused bytes.
> The second patch uses INT3 over LFENCE to shrink the retpoline to 15 bytes, by
> which 4 can live in a cacheline.
> 
> Patches have been boot tested on my IVB.

And here's the patch that prompted all this:

---
From: Borislav Petkov <bp@...e.de>
Date: Thu, 18 Feb 2021 17:21:24 +0100

Both vendors speculate after a near RET in some way:

Intel:

"Unlike near indirect CALL and near indirect JMP, the processor will not
speculatively execute the next sequential instruction after a near RET
unless that instruction is also the target of a jump or is a target in a
branch predictor."

AMD:

"Some AMD processors when they first encounter a branch do not stall
dispatch and use the branches dynamic execution to determine the target.
Therefore, they will speculatively dispatch the sequential instructions
after the branch. This happens for near return instructions where it is
not clear what code may exist sequentially after the return instruction.
This behavior also occurs with jmp/call instructions with indirect
targets. Software should place a LFENCE or another dispatch serializing
instruction after the return or jmp/call indirect instruction to prevent
this sequential speculation."

The AMD side doesn't really need the LFENCE because it'll do LFENCE;
JMP/CALL <target> due to X86_FEATURE_RETPOLINE_AMD, before it reaches
the RET.

Objtool bits provided by Peter Zijlstra (Intel) <peterz@...radead.org>

Reported-by: Rudolf Marek <r.marek@...embler.cz>
Signed-off-by: Borislav Petkov <bp@...e.de>
Co-developed-by: Peter Zijlstra (Intel) <peterz@...radead.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
---
 arch/x86/lib/retpoline.S             | 1 +
 tools/objtool/arch/x86/decode.c      | 5 +++++
 tools/objtool/check.c                | 6 ++++++
 tools/objtool/include/objtool/arch.h | 1 +
 4 files changed, 13 insertions(+)

diff --git a/arch/x86/lib/retpoline.S b/arch/x86/lib/retpoline.S
index 397d408e8244..3f8652aaf84d 100644
--- a/arch/x86/lib/retpoline.S
+++ b/arch/x86/lib/retpoline.S
@@ -31,6 +31,7 @@ SYM_FUNC_START_NOALIGN(__x86_retpoline_\reg)
 	mov	%\reg, (%_ASM_SP)
 	UNWIND_HINT_FUNC
 	ret
+	lfence
 SYM_FUNC_END(__x86_retpoline_\reg)
 
 .endm
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 549813cff8ab..84a5e3cfa72d 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -464,6 +464,11 @@ int arch_decode_instruction(const struct elf *elf, const struct section *sec,
 				op->src.type = OP_SRC_POP;
 				op->dest.type = OP_DEST_MEM;
 			}
+
+		} else if (op2 == 0xae && modrm == 0xe8) {
+
+			/* lfence */
+			*type = INSN_NOSPEC;
 		}
 
 		break;
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 331a763d8775..9ab84f0c4032 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2868,6 +2868,12 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
 	      insn->jump_dest && insn->jump_dest->type == INSN_BUG)))
 		return true;
 
+	/*
+	 * We allow speculation traps after RETURN instructions.
+	 */
+	if (prev_insn->type == INSN_RETURN && insn->type == INSN_NOSPEC)
+		return true;
+
 	/*
 	 * Check if this (or a subsequent) instruction is related to
 	 * CONFIG_UBSAN or CONFIG_KASAN.
diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
index 6ff0685f5cc5..faf0c0afd938 100644
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -26,6 +26,7 @@ enum insn_type {
 	INSN_CLAC,
 	INSN_STD,
 	INSN_CLD,
+	INSN_NOSPEC,
 	INSN_OTHER,
 };
 
-- 
2.29.2

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette