All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] crypto objtool cleanup
@ 2022-03-22 11:48 Peter Zijlstra
  2022-03-22 11:48 ` [PATCH 1/2] x86/chacha20: Avoid spurious jumps to other functions Peter Zijlstra
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Peter Zijlstra @ 2022-03-22 11:48 UTC (permalink / raw)
  To: x86
  Cc: linux-kernel, peterz, linux-crypto, ebiggers, herbert, Jason,
	Josh Poimboeuf

Hi,

two small patches that fix objtool reports on x86 crypto asm. The first
(chacha) is independent and could go through the crypto tree if desired. The
second is a fix for a commit that currently still lives in tip/x86/core and
should go there.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] x86/chacha20: Avoid spurious jumps to other functions
  2022-03-22 11:48 [PATCH 0/2] crypto objtool cleanup Peter Zijlstra
@ 2022-03-22 11:48 ` Peter Zijlstra
  2022-03-22 12:18   ` Martin Willi
  2022-03-25  4:22   ` Herbert Xu
  2022-03-22 11:48 ` [PATCH 2/2] objtool: Fix IBT tail-call detection Peter Zijlstra
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 12+ messages in thread
From: Peter Zijlstra @ 2022-03-22 11:48 UTC (permalink / raw)
  To: x86
  Cc: linux-kernel, peterz, linux-crypto, ebiggers, herbert, Jason,
	Josh Poimboeuf, Stephen Rothwell

The chacha_Nblock_xor_avx512vl() functions all have their own,
identical, .LdoneN label, however in one particular spot {2,4} jump to
the 8 version instead of their own. Resulting in:

  arch/x86/crypto/chacha-x86_64.o: warning: objtool: chacha_2block_xor_avx512vl() falls through to next function chacha_8block_xor_avx512vl()
  arch/x86/crypto/chacha-x86_64.o: warning: objtool: chacha_4block_xor_avx512vl() falls through to next function chacha_8block_xor_avx512vl()

Make each function consistently use its own done label.

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/crypto/chacha-avx512vl-x86_64.S |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/arch/x86/crypto/chacha-avx512vl-x86_64.S
+++ b/arch/x86/crypto/chacha-avx512vl-x86_64.S
@@ -172,7 +172,7 @@ SYM_FUNC_START(chacha_2block_xor_avx512v
 	# xor remaining bytes from partial register into output
 	mov		%rcx,%rax
 	and		$0xf,%rcx
-	jz		.Ldone8
+	jz		.Ldone2
 	mov		%rax,%r9
 	and		$~0xf,%r9
 
@@ -438,7 +438,7 @@ SYM_FUNC_START(chacha_4block_xor_avx512v
 	# xor remaining bytes from partial register into output
 	mov		%rcx,%rax
 	and		$0xf,%rcx
-	jz		.Ldone8
+	jz		.Ldone4
 	mov		%rax,%r9
 	and		$~0xf,%r9
 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/2] objtool: Fix IBT tail-call detection
  2022-03-22 11:48 [PATCH 0/2] crypto objtool cleanup Peter Zijlstra
  2022-03-22 11:48 ` [PATCH 1/2] x86/chacha20: Avoid spurious jumps to other functions Peter Zijlstra
@ 2022-03-22 11:48 ` Peter Zijlstra
  2022-04-05  8:29   ` [tip: x86/urgent] " tip-bot2 for Peter Zijlstra
  2022-03-23 23:05 ` [PATCH 3/2] x86/poly1305: Fixup SLS Peter Zijlstra
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Peter Zijlstra @ 2022-03-22 11:48 UTC (permalink / raw)
  To: x86
  Cc: linux-kernel, peterz, linux-crypto, ebiggers, herbert, Jason,
	Josh Poimboeuf, Stephen Rothwell

Objtool reports:

  arch/x86/crypto/poly1305-x86_64.o: warning: objtool: poly1305_blocks_avx() falls through to next function poly1305_blocks_x86_64()
  arch/x86/crypto/poly1305-x86_64.o: warning: objtool: poly1305_emit_avx() falls through to next function poly1305_emit_x86_64()
  arch/x86/crypto/poly1305-x86_64.o: warning: objtool: poly1305_blocks_avx2() falls through to next function poly1305_blocks_x86_64()

Which reads like:

0000000000000040 <poly1305_blocks_x86_64>:
	 40:       f3 0f 1e fa             endbr64
	...

0000000000000400 <poly1305_blocks_avx>:
	400:       f3 0f 1e fa             endbr64
	404:       44 8b 47 14             mov    0x14(%rdi),%r8d
	408:       48 81 fa 80 00 00 00    cmp    $0x80,%rdx
	40f:       73 09                   jae    41a <poly1305_blocks_avx+0x1a>
	411:       45 85 c0                test   %r8d,%r8d
	414:       0f 84 2a fc ff ff       je     44 <poly1305_blocks_x86_64+0x4>
	...

These are simple conditional tail-calls and *should* be recognised as
such by objtool, however due to a mistake in commit 08f87a93c8ec
("objtool: Validate IBT assumptions") this is failing.

Specifically, the jump_dest is +4, this means the instruction pointed
at will not be ENDBR and as such it will fail the second clause of
is_first_func_insn() that was supposed to capture this exact case.

Instead, have is_first_func_insn() look at the previous instruction.

Fixes: 08f87a93c8ec ("objtool: Validate IBT assumptions")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/check.c |   19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1239,11 +1239,20 @@ static bool same_function(struct instruc
 	return insn1->func->pfunc == insn2->func->pfunc;
 }
 
-static bool is_first_func_insn(struct instruction *insn)
+static bool is_first_func_insn(struct objtool_file *file, struct instruction *insn)
 {
-	return insn->offset == insn->func->offset ||
-	       (insn->type == INSN_ENDBR &&
-		insn->offset == insn->func->offset + insn->len);
+	if (insn->offset == insn->func->offset)
+		return true;
+
+	if (ibt) {
+		struct instruction *prev = prev_insn_same_sym(file, insn);
+
+		if (prev && prev->type == INSN_ENDBR &&
+		    insn->offset == insn->func->offset + prev->len)
+			return true;
+	}
+
+	return false;
 }
 
 /*
@@ -1327,7 +1336,7 @@ static int add_jump_destinations(struct
 				insn->jump_dest->func->pfunc = insn->func;
 
 			} else if (!same_function(insn, insn->jump_dest) &&
-				   is_first_func_insn(insn->jump_dest)) {
+				   is_first_func_insn(file, insn->jump_dest)) {
 				/* internal sibling call (without reloc) */
 				add_call_dest(file, insn, insn->jump_dest->func, true);
 			}



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] x86/chacha20: Avoid spurious jumps to other functions
  2022-03-22 11:48 ` [PATCH 1/2] x86/chacha20: Avoid spurious jumps to other functions Peter Zijlstra
@ 2022-03-22 12:18   ` Martin Willi
  2022-03-25  4:22   ` Herbert Xu
  1 sibling, 0 replies; 12+ messages in thread
From: Martin Willi @ 2022-03-22 12:18 UTC (permalink / raw)
  To: Peter Zijlstra, x86
  Cc: linux-kernel, linux-crypto, ebiggers, herbert, Jason,
	Josh Poimboeuf, Stephen Rothwell


> The chacha_Nblock_xor_avx512vl() functions all have their own,
> identical, .LdoneN label, however in one particular spot {2,4} jump
> to the 8 version instead of their own. [...]
> 
> Make each function consistently use its own done label.

Reviewed-by: Martin Willi <martin@strongswan.org>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 3/2] x86/poly1305: Fixup SLS
  2022-03-22 11:48 [PATCH 0/2] crypto objtool cleanup Peter Zijlstra
  2022-03-22 11:48 ` [PATCH 1/2] x86/chacha20: Avoid spurious jumps to other functions Peter Zijlstra
  2022-03-22 11:48 ` [PATCH 2/2] objtool: Fix IBT tail-call detection Peter Zijlstra
@ 2022-03-23 23:05 ` Peter Zijlstra
  2022-03-25  4:22   ` Herbert Xu
  2022-03-23 23:07 ` [PATCH 4/2] objtool: Fix SLS validation for KCOV tail-call replacement Peter Zijlstra
  2022-03-25 12:30 ` [PATCH 5/2] x86/sm3: Fixup SLS Peter Zijlstra
  4 siblings, 1 reply; 12+ messages in thread
From: Peter Zijlstra @ 2022-03-23 23:05 UTC (permalink / raw)
  To: x86; +Cc: linux-kernel, linux-crypto, ebiggers, herbert, Jason, Josh Poimboeuf


Due to being a perl generated asm file, it got missed by the mass
convertion script.

arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_init_x86_64()+0x3a: missing int3 after ret
arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_blocks_x86_64()+0xf2: missing int3 after ret
arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_emit_x86_64()+0x37: missing int3 after ret
arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: __poly1305_block()+0x6d: missing int3 after ret
arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: __poly1305_init_avx()+0x1e8: missing int3 after ret
arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_blocks_avx()+0x18a: missing int3 after ret
arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_blocks_avx()+0xaf8: missing int3 after ret
arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_emit_avx()+0x99: missing int3 after ret
arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_blocks_avx2()+0x18a: missing int3 after ret
arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_blocks_avx2()+0x776: missing int3 after ret
arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_blocks_avx512()+0x18a: missing int3 after ret
arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_blocks_avx512()+0x796: missing int3 after ret
arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_blocks_avx512()+0x10bd: missing int3 after ret

Fixes: f94909ceb1ed ("x86: Prepare asm files for straight-line-speculation")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/crypto/poly1305-x86_64-cryptogams.pl |   38 +++++++++++++-------------
 1 file changed, 19 insertions(+), 19 deletions(-)

--- a/arch/x86/crypto/poly1305-x86_64-cryptogams.pl
+++ b/arch/x86/crypto/poly1305-x86_64-cryptogams.pl
@@ -297,7 +297,7 @@ ___
 $code.=<<___;
 	mov	\$1,%eax
 .Lno_key:
-	ret
+	RET
 ___
 &end_function("poly1305_init_x86_64");
 
@@ -373,7 +373,7 @@ $code.=<<___;
 .cfi_adjust_cfa_offset	-48
 .Lno_data:
 .Lblocks_epilogue:
-	ret
+	RET
 .cfi_endproc
 ___
 &end_function("poly1305_blocks_x86_64");
@@ -399,7 +399,7 @@ $code.=<<___;
 	mov	%rax,0($mac)	# write result
 	mov	%rcx,8($mac)
 
-	ret
+	RET
 ___
 &end_function("poly1305_emit_x86_64");
 if ($avx) {
@@ -429,7 +429,7 @@ ___
 	&poly1305_iteration();
 $code.=<<___;
 	pop $ctx
-	ret
+	RET
 .size	__poly1305_block,.-__poly1305_block
 
 .type	__poly1305_init_avx,\@abi-omnipotent
@@ -594,7 +594,7 @@ $code.=<<___;
 
 	lea	-48-64($ctx),$ctx	# size [de-]optimization
 	pop %rbp
-	ret
+	RET
 .size	__poly1305_init_avx,.-__poly1305_init_avx
 ___
 
@@ -747,7 +747,7 @@ $code.=<<___;
 .cfi_restore	%rbp
 .Lno_data_avx:
 .Lblocks_avx_epilogue:
-	ret
+	RET
 .cfi_endproc
 
 .align	32
@@ -1452,7 +1452,7 @@ $code.=<<___	if (!$win64);
 ___
 $code.=<<___;
 	vzeroupper
-	ret
+	RET
 .cfi_endproc
 ___
 &end_function("poly1305_blocks_avx");
@@ -1508,7 +1508,7 @@ $code.=<<___;
 	mov	%rax,0($mac)	# write result
 	mov	%rcx,8($mac)
 
-	ret
+	RET
 ___
 &end_function("poly1305_emit_avx");
 
@@ -1675,7 +1675,7 @@ $code.=<<___;
 .cfi_restore 	%rbp
 .Lno_data_avx2$suffix:
 .Lblocks_avx2_epilogue$suffix:
-	ret
+	RET
 .cfi_endproc
 
 .align	32
@@ -2201,7 +2201,7 @@ $code.=<<___	if (!$win64);
 ___
 $code.=<<___;
 	vzeroupper
-	ret
+	RET
 .cfi_endproc
 ___
 if($avx > 2 && $avx512) {
@@ -2792,7 +2792,7 @@ $code.=<<___	if (!$win64);
 .cfi_def_cfa_register	%rsp
 ___
 $code.=<<___;
-	ret
+	RET
 .cfi_endproc
 ___
 
@@ -2893,7 +2893,7 @@ $code.=<<___	if ($flavour =~ /elf32/);
 ___
 $code.=<<___;
 	mov	\$1,%eax
-	ret
+	RET
 .size	poly1305_init_base2_44,.-poly1305_init_base2_44
 ___
 {
@@ -3010,7 +3010,7 @@ $code.=<<___;
 	jnz		.Lblocks_vpmadd52_4x
 
 .Lno_data_vpmadd52:
-	ret
+	RET
 .size	poly1305_blocks_vpmadd52,.-poly1305_blocks_vpmadd52
 ___
 }
@@ -3451,7 +3451,7 @@ $code.=<<___;
 	vzeroall
 
 .Lno_data_vpmadd52_4x:
-	ret
+	RET
 .size	poly1305_blocks_vpmadd52_4x,.-poly1305_blocks_vpmadd52_4x
 ___
 }
@@ -3824,7 +3824,7 @@ $code.=<<___;
 	vzeroall
 
 .Lno_data_vpmadd52_8x:
-	ret
+	RET
 .size	poly1305_blocks_vpmadd52_8x,.-poly1305_blocks_vpmadd52_8x
 ___
 }
@@ -3861,7 +3861,7 @@ $code.=<<___;
 	mov	%rax,0($mac)	# write result
 	mov	%rcx,8($mac)
 
-	ret
+	RET
 .size	poly1305_emit_base2_44,.-poly1305_emit_base2_44
 ___
 }	}	}
@@ -3916,7 +3916,7 @@ $code.=<<___;
 
 .Ldone_enc:
 	mov	$otp,%rax
-	ret
+	RET
 .size	xor128_encrypt_n_pad,.-xor128_encrypt_n_pad
 
 .globl	xor128_decrypt_n_pad
@@ -3967,7 +3967,7 @@ $code.=<<___;
 
 .Ldone_dec:
 	mov	$otp,%rax
-	ret
+	RET
 .size	xor128_decrypt_n_pad,.-xor128_decrypt_n_pad
 ___
 }
@@ -4109,7 +4109,7 @@ $code.=<<___;
 	pop	%rbx
 	pop	%rdi
 	pop	%rsi
-	ret
+	RET
 .size	avx_handler,.-avx_handler
 
 .section	.pdata

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 4/2] objtool: Fix SLS validation for KCOV tail-call replacement
  2022-03-22 11:48 [PATCH 0/2] crypto objtool cleanup Peter Zijlstra
                   ` (2 preceding siblings ...)
  2022-03-23 23:05 ` [PATCH 3/2] x86/poly1305: Fixup SLS Peter Zijlstra
@ 2022-03-23 23:07 ` Peter Zijlstra
  2022-04-05  8:29   ` [tip: x86/urgent] objtool: Fix SLS validation for kcov " tip-bot2 for Peter Zijlstra
  2022-03-25 12:30 ` [PATCH 5/2] x86/sm3: Fixup SLS Peter Zijlstra
  4 siblings, 1 reply; 12+ messages in thread
From: Peter Zijlstra @ 2022-03-23 23:07 UTC (permalink / raw)
  To: x86; +Cc: linux-kernel, linux-crypto, ebiggers, herbert, Jason, Josh Poimboeuf


Since not all compilers have a function attribute to disable KCOV
instrumentation, objtool can rewrite KCOV instrumentation in noinstr
functions as per commit:

  f56dae88a81f ("objtool: Handle __sanitize_cov*() tail calls")

However, this has subtle interaction with the SLS validation from
commit:

  1cc1e4c8aab4 ("objtool: Add straight-line-speculation validation")

In that when a tail-call instrucion is replaced with a RET an
additional INT3 instruction is also written, but is not represented in
the decoded instruction stream.

This then leads to false positive missing INT3 objtool warnings in
noinstr code.

Instead of adding additional struct instruction objects, mark the RET
instruction with retpoline_safe to suppress the warning (since we know
there really is an INT3).

Fixes: 1cc1e4c8aab4 ("objtool: Add straight-line-speculation validation")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 tools/objtool/check.c |   43 +++++++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 14 deletions(-)

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1090,6 +1099,17 @@ static void annotate_call_site(struct ob
 			               : arch_nop_insn(insn->len));
 
 		insn->type = sibling ? INSN_RETURN : INSN_NOP;
+
+		if (sibling) {
+			/*
+			 * We've replaced the tail-call JMP insn by two new
+			 * insn: RET; INT3, except we only have a single struct
+			 * insn here. Mark it retpoline_safe to avoid the SLS
+			 * warning, instead of adding another insn.
+			 */
+			insn->retpoline_safe = true;
+		}
+
 		return;
 	}
 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] x86/chacha20: Avoid spurious jumps to other functions
  2022-03-22 11:48 ` [PATCH 1/2] x86/chacha20: Avoid spurious jumps to other functions Peter Zijlstra
  2022-03-22 12:18   ` Martin Willi
@ 2022-03-25  4:22   ` Herbert Xu
  1 sibling, 0 replies; 12+ messages in thread
From: Herbert Xu @ 2022-03-25  4:22 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: x86, linux-kernel, linux-crypto, ebiggers, Jason, Josh Poimboeuf,
	Stephen Rothwell

On Tue, Mar 22, 2022 at 12:48:10PM +0100, Peter Zijlstra wrote:
> The chacha_Nblock_xor_avx512vl() functions all have their own,
> identical, .LdoneN label, however in one particular spot {2,4} jump to
> the 8 version instead of their own. Resulting in:
> 
>   arch/x86/crypto/chacha-x86_64.o: warning: objtool: chacha_2block_xor_avx512vl() falls through to next function chacha_8block_xor_avx512vl()
>   arch/x86/crypto/chacha-x86_64.o: warning: objtool: chacha_4block_xor_avx512vl() falls through to next function chacha_8block_xor_avx512vl()
> 
> Make each function consistently use its own done label.
> 
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  arch/x86/crypto/chacha-avx512vl-x86_64.S |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/2] x86/poly1305: Fixup SLS
  2022-03-23 23:05 ` [PATCH 3/2] x86/poly1305: Fixup SLS Peter Zijlstra
@ 2022-03-25  4:22   ` Herbert Xu
  0 siblings, 0 replies; 12+ messages in thread
From: Herbert Xu @ 2022-03-25  4:22 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: x86, linux-kernel, linux-crypto, ebiggers, Jason, Josh Poimboeuf

On Thu, Mar 24, 2022 at 12:05:55AM +0100, Peter Zijlstra wrote:
> 
> Due to being a perl generated asm file, it got missed by the mass
> convertion script.
> 
> arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_init_x86_64()+0x3a: missing int3 after ret
> arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_blocks_x86_64()+0xf2: missing int3 after ret
> arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_emit_x86_64()+0x37: missing int3 after ret
> arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: __poly1305_block()+0x6d: missing int3 after ret
> arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: __poly1305_init_avx()+0x1e8: missing int3 after ret
> arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_blocks_avx()+0x18a: missing int3 after ret
> arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_blocks_avx()+0xaf8: missing int3 after ret
> arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_emit_avx()+0x99: missing int3 after ret
> arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_blocks_avx2()+0x18a: missing int3 after ret
> arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_blocks_avx2()+0x776: missing int3 after ret
> arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_blocks_avx512()+0x18a: missing int3 after ret
> arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_blocks_avx512()+0x796: missing int3 after ret
> arch/x86/crypto/poly1305-x86_64-cryptogams.o: warning: objtool: poly1305_blocks_avx512()+0x10bd: missing int3 after ret
> 
> Fixes: f94909ceb1ed ("x86: Prepare asm files for straight-line-speculation")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  arch/x86/crypto/poly1305-x86_64-cryptogams.pl |   38 +++++++++++++-------------
>  1 file changed, 19 insertions(+), 19 deletions(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 5/2] x86/sm3: Fixup SLS
  2022-03-22 11:48 [PATCH 0/2] crypto objtool cleanup Peter Zijlstra
                   ` (3 preceding siblings ...)
  2022-03-23 23:07 ` [PATCH 4/2] objtool: Fix SLS validation for KCOV tail-call replacement Peter Zijlstra
@ 2022-03-25 12:30 ` Peter Zijlstra
  2022-03-30  5:19   ` Herbert Xu
  4 siblings, 1 reply; 12+ messages in thread
From: Peter Zijlstra @ 2022-03-25 12:30 UTC (permalink / raw)
  To: x86; +Cc: linux-kernel, linux-crypto, ebiggers, herbert, Jason, Josh Poimboeuf


This missed the big asm update due to being merged through the crypto
tree.

Fixes: f94909ceb1ed ("x86: Prepare asm files for straight-line-speculation")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/crypto/sm3-avx-asm_64.S |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/x86/crypto/sm3-avx-asm_64.S
+++ b/arch/x86/crypto/sm3-avx-asm_64.S
@@ -513,5 +513,5 @@ SYM_FUNC_START(sm3_transform_avx)
 
 	movq %rbp, %rsp;
 	popq %rbp;
-	ret;
+	RET;
 SYM_FUNC_END(sm3_transform_avx)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/2] x86/sm3: Fixup SLS
  2022-03-25 12:30 ` [PATCH 5/2] x86/sm3: Fixup SLS Peter Zijlstra
@ 2022-03-30  5:19   ` Herbert Xu
  0 siblings, 0 replies; 12+ messages in thread
From: Herbert Xu @ 2022-03-30  5:19 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: x86, linux-kernel, linux-crypto, ebiggers, Jason, Josh Poimboeuf

On Fri, Mar 25, 2022 at 01:30:47PM +0100, Peter Zijlstra wrote:
> 
> This missed the big asm update due to being merged through the crypto
> tree.
> 
> Fixes: f94909ceb1ed ("x86: Prepare asm files for straight-line-speculation")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  arch/x86/crypto/sm3-avx-asm_64.S |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [tip: x86/urgent] objtool: Fix SLS validation for kcov tail-call replacement
  2022-03-23 23:07 ` [PATCH 4/2] objtool: Fix SLS validation for KCOV tail-call replacement Peter Zijlstra
@ 2022-04-05  8:29   ` tip-bot2 for Peter Zijlstra
  0 siblings, 0 replies; 12+ messages in thread
From: tip-bot2 for Peter Zijlstra @ 2022-04-05  8:29 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Peter Zijlstra (Intel), x86, linux-kernel

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     7a53f408902d913cd541b4f8ad7dbcd4961f5b82
Gitweb:        https://git.kernel.org/tip/7a53f408902d913cd541b4f8ad7dbcd4961f5b82
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Wed, 23 Mar 2022 23:35:01 +01:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 05 Apr 2022 10:24:40 +02:00

objtool: Fix SLS validation for kcov tail-call replacement

Since not all compilers have a function attribute to disable KCOV
instrumentation, objtool can rewrite KCOV instrumentation in noinstr
functions as per commit:

  f56dae88a81f ("objtool: Handle __sanitize_cov*() tail calls")

However, this has subtle interaction with the SLS validation from
commit:

  1cc1e4c8aab4 ("objtool: Add straight-line-speculation validation")

In that when a tail-call instrucion is replaced with a RET an
additional INT3 instruction is also written, but is not represented in
the decoded instruction stream.

This then leads to false positive missing INT3 objtool warnings in
noinstr code.

Instead of adding additional struct instruction objects, mark the RET
instruction with retpoline_safe to suppress the warning (since we know
there really is an INT3).

Fixes: 1cc1e4c8aab4 ("objtool: Add straight-line-speculation validation")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20220323230712.GA8939@worktop.programming.kicks-ass.net
---
 tools/objtool/check.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index b848e1d..bd0c2c8 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1155,6 +1155,17 @@ static void annotate_call_site(struct objtool_file *file,
 			               : arch_nop_insn(insn->len));
 
 		insn->type = sibling ? INSN_RETURN : INSN_NOP;
+
+		if (sibling) {
+			/*
+			 * We've replaced the tail-call JMP insn by two new
+			 * insn: RET; INT3, except we only have a single struct
+			 * insn here. Mark it retpoline_safe to avoid the SLS
+			 * warning, instead of adding another insn.
+			 */
+			insn->retpoline_safe = true;
+		}
+
 		return;
 	}
 

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [tip: x86/urgent] objtool: Fix IBT tail-call detection
  2022-03-22 11:48 ` [PATCH 2/2] objtool: Fix IBT tail-call detection Peter Zijlstra
@ 2022-04-05  8:29   ` tip-bot2 for Peter Zijlstra
  0 siblings, 0 replies; 12+ messages in thread
From: tip-bot2 for Peter Zijlstra @ 2022-04-05  8:29 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Stephen Rothwell, Peter Zijlstra (Intel), x86, linux-kernel

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     d139bca4b824ffb9731763c31b271a24b595948a
Gitweb:        https://git.kernel.org/tip/d139bca4b824ffb9731763c31b271a24b595948a
Author:        Peter Zijlstra <peterz@infradead.org>
AuthorDate:    Tue, 22 Mar 2022 12:33:31 +01:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Tue, 05 Apr 2022 10:24:40 +02:00

objtool: Fix IBT tail-call detection

Objtool reports:

  arch/x86/crypto/poly1305-x86_64.o: warning: objtool: poly1305_blocks_avx() falls through to next function poly1305_blocks_x86_64()
  arch/x86/crypto/poly1305-x86_64.o: warning: objtool: poly1305_emit_avx() falls through to next function poly1305_emit_x86_64()
  arch/x86/crypto/poly1305-x86_64.o: warning: objtool: poly1305_blocks_avx2() falls through to next function poly1305_blocks_x86_64()

Which reads like:

0000000000000040 <poly1305_blocks_x86_64>:
	 40:       f3 0f 1e fa             endbr64
	...

0000000000000400 <poly1305_blocks_avx>:
	400:       f3 0f 1e fa             endbr64
	404:       44 8b 47 14             mov    0x14(%rdi),%r8d
	408:       48 81 fa 80 00 00 00    cmp    $0x80,%rdx
	40f:       73 09                   jae    41a <poly1305_blocks_avx+0x1a>
	411:       45 85 c0                test   %r8d,%r8d
	414:       0f 84 2a fc ff ff       je     44 <poly1305_blocks_x86_64+0x4>
	...

These are simple conditional tail-calls and *should* be recognised as
such by objtool, however due to a mistake in commit 08f87a93c8ec
("objtool: Validate IBT assumptions") this is failing.

Specifically, the jump_dest is +4, this means the instruction pointed
at will not be ENDBR and as such it will fail the second clause of
is_first_func_insn() that was supposed to capture this exact case.

Instead, have is_first_func_insn() look at the previous instruction.

Fixes: 08f87a93c8ec ("objtool: Validate IBT assumptions")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20220322115125.811582125@infradead.org
---
 tools/objtool/check.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 6de5085..b848e1d 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1239,11 +1239,20 @@ static bool same_function(struct instruction *insn1, struct instruction *insn2)
 	return insn1->func->pfunc == insn2->func->pfunc;
 }
 
-static bool is_first_func_insn(struct instruction *insn)
+static bool is_first_func_insn(struct objtool_file *file, struct instruction *insn)
 {
-	return insn->offset == insn->func->offset ||
-	       (insn->type == INSN_ENDBR &&
-		insn->offset == insn->func->offset + insn->len);
+	if (insn->offset == insn->func->offset)
+		return true;
+
+	if (ibt) {
+		struct instruction *prev = prev_insn_same_sym(file, insn);
+
+		if (prev && prev->type == INSN_ENDBR &&
+		    insn->offset == insn->func->offset + prev->len)
+			return true;
+	}
+
+	return false;
 }
 
 /*
@@ -1327,7 +1336,7 @@ static int add_jump_destinations(struct objtool_file *file)
 				insn->jump_dest->func->pfunc = insn->func;
 
 			} else if (!same_function(insn, insn->jump_dest) &&
-				   is_first_func_insn(insn->jump_dest)) {
+				   is_first_func_insn(file, insn->jump_dest)) {
 				/* internal sibling call (without reloc) */
 				add_call_dest(file, insn, insn->jump_dest->func, true);
 			}

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-04-05 10:30 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-22 11:48 [PATCH 0/2] crypto objtool cleanup Peter Zijlstra
2022-03-22 11:48 ` [PATCH 1/2] x86/chacha20: Avoid spurious jumps to other functions Peter Zijlstra
2022-03-22 12:18   ` Martin Willi
2022-03-25  4:22   ` Herbert Xu
2022-03-22 11:48 ` [PATCH 2/2] objtool: Fix IBT tail-call detection Peter Zijlstra
2022-04-05  8:29   ` [tip: x86/urgent] " tip-bot2 for Peter Zijlstra
2022-03-23 23:05 ` [PATCH 3/2] x86/poly1305: Fixup SLS Peter Zijlstra
2022-03-25  4:22   ` Herbert Xu
2022-03-23 23:07 ` [PATCH 4/2] objtool: Fix SLS validation for KCOV tail-call replacement Peter Zijlstra
2022-04-05  8:29   ` [tip: x86/urgent] objtool: Fix SLS validation for kcov " tip-bot2 for Peter Zijlstra
2022-03-25 12:30 ` [PATCH 5/2] x86/sm3: Fixup SLS Peter Zijlstra
2022-03-30  5:19   ` Herbert Xu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.