[PATCH bpf-next] bpf: Clear zext_dst of dead insns

[PATCH bpf-next] bpf: Clear zext_dst of dead insns

[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.

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 5ea2238a6656..e5f2b23bb7c9 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -11955,6 +11955,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

Re: [PATCH bpf-next] bpf: Clear zext_dst of dead insns

[Daniel Borkmann]

Hey Ilya,

On 8/12/21 1:12 PM, Ilya Leoshkevich wrote:
> "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.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>

I presume this would rather be bpf tree material, no? Do you also have a
Fixes tag I could add?

And one last small request: if this is not already covered by test_verifier
selftest, could you add one along with the fix?

Thanks a lot,
Daniel