* [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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ 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; 13+ 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] 13+ messages in thread