linux-kernel - Re: [RFC] x86/alternative: Support relocations in alternatives

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]

Date:   Wed, 1 Feb 2023 20:02:31 +0300
From:   "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     x86@...nel.org, Borislav Petkov <bp@...en8.de>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Andrew Cooper <Andrew.Cooper3@...rix.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC] x86/alternative: Support relocations in alternatives

On Wed, Feb 01, 2023 at 03:10:33PM +0100, Peter Zijlstra wrote:
> +static void __init_or_module noinline
> +apply_relocation(u8 *instr, size_t len, u8 *dest, u8 *src, size_t src_len)
> +{
> +	struct insn insn;
> +	int i = 0;
> +
> +	for (;;) {
> +		if (insn_decode_kernel(&insn, &instr[i]))
> +			return;
> +
> +		switch (insn.opcode.bytes[0]) {
> +		case 0x0f:
> +			if (insn.opcode.bytes[1] < 0x80 ||
> +			    insn.opcode.bytes[1] > 0x8f)
> +				break;
> +
> +			fallthrough;	/* Jcc.d32 */
> +		case 0x70 ... 0x7f:	/* Jcc.d8 */
> +		case JMP8_INSN_OPCODE:
> +		case JMP32_INSN_OPCODE:
> +		case CALL_INSN_OPCODE:
> +			u8 *target = src + i + insn.length + insn.immediate.value;

LLVM doesn't like this declaration of 'target'. I've moved to the top of
the function.

I've tested the patch with both GCC and LLVM (after 'target' fix) with my
LAM case. See the patch below.

Works fine. Thanks.

Tested-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>

diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index 6450a2723bcd..ec7613141db6 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -35,17 +35,12 @@ static inline unsigned long __untagged_addr(unsigned long addr)
 {
 	long sign;
 
-	/*
-	 * Refer tlbstate_untag_mask directly to avoid RIP-relative relocation
-	 * in alternative instructions. The relocation gets wrong when gets
-	 * copied to the target place.
-	 */
 	asm (ALTERNATIVE("",
 			 "sar $63, %[sign]\n\t" /* user_ptr ? 0 : -1UL */
-			 "or %%gs:tlbstate_untag_mask, %[sign]\n\t"
+			 "or " __percpu_arg([mask]) ", %[sign]\n\t"
 			 "and %[sign], %[addr]\n\t", X86_FEATURE_LAM)
 	     : [addr] "+r" (addr), [sign] "=r" (sign)
-	     : "m" (tlbstate_untag_mask), "[sign]" (addr));
+	     : [mask] "m" (tlbstate_untag_mask), "[sign]" (addr));
 
 	return addr;
 }
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 50b88aa3ebee..bb1437ac2f5a 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -212,6 +212,7 @@ static void __init_or_module noinline
 apply_relocation(u8 *instr, size_t len, u8 *dest, u8 *src, size_t src_len)
 {
 	struct insn insn;
+	u8 *target;
 	int i = 0;
 
 	for (;;) {
@@ -229,7 +230,7 @@ apply_relocation(u8 *instr, size_t len, u8 *dest, u8 *src, size_t src_len)
 		case JMP8_INSN_OPCODE:
 		case JMP32_INSN_OPCODE:
 		case CALL_INSN_OPCODE:
-			u8 *target = src + i + insn.length + insn.immediate.value;
+			target = src + i + insn.length + insn.immediate.value;
 			if (target < src || target > src + src_len) {
 				apply_reloc(insn.immediate.nbytes,
 					    instr + i + insn_offset_immediate(&insn),
-- 
  Kiryl Shutsemau / Kirill A. Shutemov