| Message ID | 20210812111220.181824-1-iii@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] bpf: Clear zext_dst of dead insns | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 7 maintainers not CCed: netdev@vger.kernel.org yhs@fb.com john.fastabend@gmail.com songliubraving@fb.com andrii@kernel.org kpsingh@kernel.org kafai@fb.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 30 this patch: 30 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 7 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 30 this patch: 30 |
| netdev/header_inline | success | Link |
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
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; } }
"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(+)