* [PATCH bpf v3 1/2] bpf: clear zext_dst of dead insns
2021-08-12 15:18 [PATCH bpf v3 0/2] selftests: bpf: test that dead ldx_w insns are accepted Ilya Leoshkevich
@ 2021-08-12 15:18 ` Ilya Leoshkevich
2021-08-12 15:18 ` [PATCH bpf v3 2/2] selftests: bpf: test that dead ldx_w insns are accepted Ilya Leoshkevich
2021-08-13 15:50 ` [PATCH bpf v3 0/2] " patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: Ilya Leoshkevich @ 2021-08-12 15:18 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich
"access skb fields ok" verifier test fails on s390 with the "verifier
bug. zext_dst is set, but no reg is defined" message. The first insns
of the test prog are:
0: 61 01 00 00 00 00 00 00 ldxw %r0,[%r1+0]
8: 35 00 00 01 00 00 00 00 jge %r0,0,1
10: 61 01 00 08 00 00 00 00 ldxw %r0,[%r1+8]
and the 3rd one is dead (this does not look intentional to me, but this
is a separate topic).
sanitize_dead_code() converts dead insns into "ja -1", but keeps
zext_dst. When opt_subreg_zext_lo32_rnd_hi32() tries to parse such
an insn, it sees this discrepancy and bails. This problem can be seen
only with JITs whose bpf_jit_needs_zext() returns true.
Fix by clearning dead insns' zext_dst.
The commits that contributed to this problem are:
1. commit 5aa5bd14c5f8 ("bpf: add initial suite for selftests"), which
introduced the test with the dead code.
2. commit 5327ed3d44b7 ("bpf: verifier: mark verified-insn with
sub-register zext flag"), which introduced the zext_dst flag.
3. commit 83a2881903f3 ("bpf: Account for BPF_FETCH in
insn_has_def32()"), which introduced the sanity check.
4. commit 9183671af6db ("bpf: Fix leakage under speculation on
mispredicted branches"), which bisect points to.
It's best to fix this on stable branches that contain the second one,
since that's the point where the inconsistency was introduced.
Fixes: 5327ed3d44b7 ("bpf: verifier: mark verified-insn with sub-register zext flag")
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
kernel/bpf/verifier.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index f9bda5476ea5..381d3d6f24bc 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -11663,6 +11663,7 @@ static void sanitize_dead_code(struct bpf_verifier_env *env)
if (aux_data[i].seen)
continue;
memcpy(insn + i, &trap, sizeof(trap));
+ aux_data[i].zext_dst = false;
}
}
--
2.31.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH bpf v3 2/2] selftests: bpf: test that dead ldx_w insns are accepted
2021-08-12 15:18 [PATCH bpf v3 0/2] selftests: bpf: test that dead ldx_w insns are accepted Ilya Leoshkevich
2021-08-12 15:18 ` [PATCH bpf v3 1/2] bpf: clear zext_dst of dead insns Ilya Leoshkevich
@ 2021-08-12 15:18 ` Ilya Leoshkevich
2021-08-13 15:50 ` [PATCH bpf v3 0/2] " patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: Ilya Leoshkevich @ 2021-08-12 15:18 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: bpf, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich
Prevent regressions related to zero-extension metadata handling during
dead code sanitization.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tools/testing/selftests/bpf/verifier/dead_code.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/tools/testing/selftests/bpf/verifier/dead_code.c b/tools/testing/selftests/bpf/verifier/dead_code.c
index 2c8935b3e65d..ee454327e5c6 100644
--- a/tools/testing/selftests/bpf/verifier/dead_code.c
+++ b/tools/testing/selftests/bpf/verifier/dead_code.c
@@ -159,3 +159,15 @@
.result = ACCEPT,
.retval = 2,
},
+{
+ "dead code: zero extension",
+ .insns = {
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -4),
+ BPF_JMP_IMM(BPF_JGE, BPF_REG_0, 0, 1),
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_10, -4),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .retval = 0,
+},
--
2.31.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH bpf v3 0/2] selftests: bpf: test that dead ldx_w insns are accepted
2021-08-12 15:18 [PATCH bpf v3 0/2] selftests: bpf: test that dead ldx_w insns are accepted Ilya Leoshkevich
2021-08-12 15:18 ` [PATCH bpf v3 1/2] bpf: clear zext_dst of dead insns Ilya Leoshkevich
2021-08-12 15:18 ` [PATCH bpf v3 2/2] selftests: bpf: test that dead ldx_w insns are accepted Ilya Leoshkevich
@ 2021-08-13 15:50 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-08-13 15:50 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: ast, daniel, bpf, hca, gor
Hello:
This series was applied to bpf/bpf.git (refs/heads/master):
On Thu, 12 Aug 2021 17:18:09 +0200 you wrote:
> Fix the "verifier bug. zext_dst is set, but no reg is defined" failure
> in the "access skb fields ok" test on s390.
>
> Patch 1 is the fix, patch 2 adds a test.
>
> v2: https://lore.kernel.org/bpf/20210812140518.183178-1-iii@linux.ibm.com/
> v2 -> v3: Make sure that the test fails in absence of the fix.
>
> [...]
Here is the summary with links:
- [bpf,v3,1/2] bpf: clear zext_dst of dead insns
https://git.kernel.org/bpf/bpf/c/45c709f8c71b
- [bpf,v3,2/2] selftests: bpf: test that dead ldx_w insns are accepted
https://git.kernel.org/bpf/bpf/c/3776f3517ed9
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 4+ messages in thread