Skip to content

Commit 94e88b8

Browse files
author
Alexei Starovoitov
committed
Merge branch 'bpf-fix-precision-tracking-for-bpf_alu-bpf_to_be-bpf_end'
Shung-Hsi Yu says: ==================== bpf: Fix precision tracking for BPF_ALU | BPF_TO_BE | BPF_END Changes since v1: - add test for negation and bswap (Alexei, Eduard) - add test for BPF_TO_LE as well to cover all types of BPF_END opcode - remove vals map and trigger backtracking with jump instead, based of Eduard's code - v1 at https://lore.kernel.org/bpf/20231030132145.20867-1-shung-hsi.yu@suse.com This patchset fixes and adds selftest for the issue reported by Mohamed Mahmoud and Toke Høiland-Jørgensen where the kernel can run into a verifier bug during backtracking of BPF_ALU | BPF_TO_BE | BPF_END instruction[0]. As seen in the verifier log below, r0 was incorrectly marked as precise even tough its value was not being used. Patch 1 fixes the issue based on Andrii's analysis, and patch 2 adds a selftest for such case using inline assembly. Please see individual patch for detail. ... mark_precise: frame2: regs=r2 stack= before 1891: (77) r2 >>= 56 mark_precise: frame2: regs=r2 stack= before 1890: (dc) r2 = be64 r2 mark_precise: frame2: regs=r0,r2 stack= before 1889: (73) *(u8 *)(r1 +47) = r3 ... mark_precise: frame2: regs=r0 stack= before 212: (85) call pc+1617 BUG regs 1 processed 5112 insns (limit 1000000) max_states_per_insn 4 total_states 92 peak_states 90 mark_read 20 0: https://lore.kernel.org/r/87jzrrwptf.fsf@toke.dk Shung-Hsi Yu (2): bpf: Fix precision tracking for BPF_ALU | BPF_TO_BE | BPF_END selftests/bpf: precision tracking test for BPF_NEG and BPF_END kernel/bpf/verifier.c | 7 +- .../selftests/bpf/prog_tests/verifier.c | 2 + .../selftests/bpf/progs/verifier_precision.c | 93 +++++++++++++++++++ 3 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/bpf/progs/verifier_precision.c base-commit: c17cda1 ==================== Link: https://lore.kernel.org/r/20231102053913.12004-1-shung-hsi.yu@suse.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
2 parents 698b8c5 + 3c41971 commit 94e88b8

File tree

3 files changed

+101
-1
lines changed

3 files changed

+101
-1
lines changed

‎kernel/bpf/verifier.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3742,7 +3742,12 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
37423742
if (class == BPF_ALU || class == BPF_ALU64) {
37433743
if (!bt_is_reg_set(bt, dreg))
37443744
return 0;
3745-
if (opcode == BPF_MOV) {
3745+
if (opcode == BPF_END || opcode == BPF_NEG) {
3746+
/* sreg is reserved and unused
3747+
* dreg still need precision before this insn
3748+
*/
3749+
return 0;
3750+
} else if (opcode == BPF_MOV) {
37463751
if (BPF_SRC(insn->code) == BPF_X) {
37473752
/* dreg = sreg or dreg = (s8, s16, s32)sreg
37483753
* dreg needs precision after this insn

‎tools/testing/selftests/bpf/prog_tests/verifier.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
#include "verifier_movsx.skel.h"
4747
#include "verifier_netfilter_ctx.skel.h"
4848
#include "verifier_netfilter_retcode.skel.h"
49+
#include "verifier_precision.skel.h"
4950
#include "verifier_prevent_map_lookup.skel.h"
5051
#include "verifier_raw_stack.skel.h"
5152
#include "verifier_raw_tp_writable.skel.h"
@@ -153,6 +154,7 @@ void test_verifier_meta_access(void) { RUN(verifier_meta_access); }
153154
void test_verifier_movsx(void) { RUN(verifier_movsx); }
154155
void test_verifier_netfilter_ctx(void) { RUN(verifier_netfilter_ctx); }
155156
void test_verifier_netfilter_retcode(void) { RUN(verifier_netfilter_retcode); }
157+
void test_verifier_precision(void) { RUN(verifier_precision); }
156158
void test_verifier_prevent_map_lookup(void) { RUN(verifier_prevent_map_lookup); }
157159
void test_verifier_raw_stack(void) { RUN(verifier_raw_stack); }
158160
void test_verifier_raw_tp_writable(void) { RUN(verifier_raw_tp_writable); }

‎tools/testing/selftests/bpf/progs/verifier_precision.c

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
/* Copyright (C) 2023 SUSE LLC */
3+
#include <linux/bpf.h>
4+
#include <bpf/bpf_helpers.h>
5+
#include "bpf_misc.h"
6+
7+
SEC("?raw_tp")
8+
__success __log_level(2)
9+
__msg("mark_precise: frame0: regs=r2 stack= before 3: (bf) r1 = r10")
10+
__msg("mark_precise: frame0: regs=r2 stack= before 2: (55) if r2 != 0xfffffff8 goto pc+2")
11+
__msg("mark_precise: frame0: regs=r2 stack= before 1: (87) r2 = -r2")
12+
__msg("mark_precise: frame0: regs=r2 stack= before 0: (b7) r2 = 8")
13+
__naked int bpf_neg(void)
14+
{
15+
asm volatile (
16+
"r2 = 8;"
17+
"r2 = -r2;"
18+
"if r2 != -8 goto 1f;"
19+
"r1 = r10;"
20+
"r1 += r2;"
21+
"1:"
22+
"r0 = 0;"
23+
"exit;"
24+
::: __clobber_all);
25+
}
26+
27+
SEC("?raw_tp")
28+
__success __log_level(2)
29+
__msg("mark_precise: frame0: regs=r2 stack= before 3: (bf) r1 = r10")
30+
__msg("mark_precise: frame0: regs=r2 stack= before 2: (55) if r2 != 0x0 goto pc+2")
31+
__msg("mark_precise: frame0: regs=r2 stack= before 1: (d4) r2 = le16 r2")
32+
__msg("mark_precise: frame0: regs=r2 stack= before 0: (b7) r2 = 0")
33+
__naked int bpf_end_to_le(void)
34+
{
35+
asm volatile (
36+
"r2 = 0;"
37+
"r2 = le16 r2;"
38+
"if r2 != 0 goto 1f;"
39+
"r1 = r10;"
40+
"r1 += r2;"
41+
"1:"
42+
"r0 = 0;"
43+
"exit;"
44+
::: __clobber_all);
45+
}
46+
47+
48+
SEC("?raw_tp")
49+
__success __log_level(2)
50+
__msg("mark_precise: frame0: regs=r2 stack= before 3: (bf) r1 = r10")
51+
__msg("mark_precise: frame0: regs=r2 stack= before 2: (55) if r2 != 0x0 goto pc+2")
52+
__msg("mark_precise: frame0: regs=r2 stack= before 1: (dc) r2 = be16 r2")
53+
__msg("mark_precise: frame0: regs=r2 stack= before 0: (b7) r2 = 0")
54+
__naked int bpf_end_to_be(void)
55+
{
56+
asm volatile (
57+
"r2 = 0;"
58+
"r2 = be16 r2;"
59+
"if r2 != 0 goto 1f;"
60+
"r1 = r10;"
61+
"r1 += r2;"
62+
"1:"
63+
"r0 = 0;"
64+
"exit;"
65+
::: __clobber_all);
66+
}
67+
68+
#if (defined(__TARGET_ARCH_arm64) || defined(__TARGET_ARCH_x86) || \
69+
(defined(__TARGET_ARCH_riscv) && __riscv_xlen == 64) || \
70+
defined(__TARGET_ARCH_arm) || defined(__TARGET_ARCH_s390)) && \
71+
__clang_major__ >= 18
72+
73+
SEC("?raw_tp")
74+
__success __log_level(2)
75+
__msg("mark_precise: frame0: regs=r2 stack= before 3: (bf) r1 = r10")
76+
__msg("mark_precise: frame0: regs=r2 stack= before 2: (55) if r2 != 0x0 goto pc+2")
77+
__msg("mark_precise: frame0: regs=r2 stack= before 1: (d7) r2 = bswap16 r2")
78+
__msg("mark_precise: frame0: regs=r2 stack= before 0: (b7) r2 = 0")
79+
__naked int bpf_end_bswap(void)
80+
{
81+
asm volatile (
82+
"r2 = 0;"
83+
"r2 = bswap16 r2;"
84+
"if r2 != 0 goto 1f;"
85+
"r1 = r10;"
86+
"r1 += r2;"
87+
"1:"
88+
"r0 = 0;"
89+
"exit;"
90+
::: __clobber_all);
91+
}
92+
93+
#endif /* v4 instruction */

0 commit comments

Comments
 (0)