Skip to content

Commit 98d7ca3

Browse files
Alexei Starovoitovborkmann
authored andcommitted
bpf: Track delta between "linked" registers.
Compilers can generate the code r1 = r2 r1 += 0x1 if r2 < 1000 goto ... use knowledge of r2 range in subsequent r1 operations So remember constant delta between r2 and r1 and update r1 after 'if' condition. Unfortunately LLVM still uses this pattern for loops with 'can_loop' construct: for (i = 0; i < 1000 && can_loop; i++) The "undo" pass was introduced in LLVM https://reviews.llvm.org/D121937 to prevent this optimization, but it cannot cover all cases. Instead of fighting middle end optimizer in BPF backend teach the verifier about this pattern. Signed-off-by: Alexei Starovoitov <[email protected]> Signed-off-by: Daniel Borkmann <[email protected]> Acked-by: Eduard Zingerman <[email protected]> Link: https://lore.kernel.org/bpf/[email protected]
1 parent 124e8c2 commit 98d7ca3

File tree

4 files changed

+109
-24
lines changed

4 files changed

+109
-24
lines changed

include/linux/bpf_verifier.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,10 @@ enum bpf_iter_state {
7373
struct bpf_reg_state {
7474
/* Ordering of fields matters. See states_equal() */
7575
enum bpf_reg_type type;
76-
/* Fixed part of pointer offset, pointer types only */
76+
/*
77+
* Fixed part of pointer offset, pointer types only.
78+
* Or constant delta between "linked" scalars with the same ID.
79+
*/
7780
s32 off;
7881
union {
7982
/* valid when type == PTR_TO_PACKET */
@@ -167,6 +170,13 @@ struct bpf_reg_state {
167170
* Similarly to dynptrs, we use ID to track "belonging" of a reference
168171
* to a specific instance of bpf_iter.
169172
*/
173+
/*
174+
* Upper bit of ID is used to remember relationship between "linked"
175+
* registers. Example:
176+
* r1 = r2; both will have r1->id == r2->id == N
177+
* r1 += 10; r1->id == N | BPF_ADD_CONST and r1->off == 10
178+
*/
179+
#define BPF_ADD_CONST (1U << 31)
170180
u32 id;
171181
/* PTR_TO_SOCKET and PTR_TO_TCP_SOCK could be a ptr returned
172182
* from a pointer-cast helper, bpf_sk_fullsock() and

kernel/bpf/log.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -708,7 +708,9 @@ static void print_reg_state(struct bpf_verifier_env *env,
708708
verbose(env, "%s", btf_type_name(reg->btf, reg->btf_id));
709709
verbose(env, "(");
710710
if (reg->id)
711-
verbose_a("id=%d", reg->id);
711+
verbose_a("id=%d", reg->id & ~BPF_ADD_CONST);
712+
if (reg->id & BPF_ADD_CONST)
713+
verbose(env, "%+d", reg->off);
712714
if (reg->ref_obj_id)
713715
verbose_a("ref_obj_id=%d", reg->ref_obj_id);
714716
if (type_is_non_owning_ref(reg->type))

kernel/bpf/verifier.c

Lines changed: 84 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3991,7 +3991,7 @@ static bool idset_contains(struct bpf_idset *s, u32 id)
39913991
u32 i;
39923992

39933993
for (i = 0; i < s->count; ++i)
3994-
if (s->ids[i] == id)
3994+
if (s->ids[i] == (id & ~BPF_ADD_CONST))
39953995
return true;
39963996

39973997
return false;
@@ -4001,7 +4001,7 @@ static int idset_push(struct bpf_idset *s, u32 id)
40014001
{
40024002
if (WARN_ON_ONCE(s->count >= ARRAY_SIZE(s->ids)))
40034003
return -EFAULT;
4004-
s->ids[s->count++] = id;
4004+
s->ids[s->count++] = id & ~BPF_ADD_CONST;
40054005
return 0;
40064006
}
40074007

@@ -4438,8 +4438,20 @@ static bool __is_pointer_value(bool allow_ptr_leaks,
44384438
static void assign_scalar_id_before_mov(struct bpf_verifier_env *env,
44394439
struct bpf_reg_state *src_reg)
44404440
{
4441-
if (src_reg->type == SCALAR_VALUE && !src_reg->id &&
4442-
!tnum_is_const(src_reg->var_off))
4441+
if (src_reg->type != SCALAR_VALUE)
4442+
return;
4443+
4444+
if (src_reg->id & BPF_ADD_CONST) {
4445+
/*
4446+
* The verifier is processing rX = rY insn and
4447+
* rY->id has special linked register already.
4448+
* Cleared it, since multiple rX += const are not supported.
4449+
*/
4450+
src_reg->id = 0;
4451+
src_reg->off = 0;
4452+
}
4453+
4454+
if (!src_reg->id && !tnum_is_const(src_reg->var_off))
44434455
/* Ensure that src_reg has a valid ID that will be copied to
44444456
* dst_reg and then will be used by find_equal_scalars() to
44454457
* propagate min/max range.
@@ -14042,6 +14054,7 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env,
1404214054
struct bpf_func_state *state = vstate->frame[vstate->curframe];
1404314055
struct bpf_reg_state *regs = state->regs, *dst_reg, *src_reg;
1404414056
struct bpf_reg_state *ptr_reg = NULL, off_reg = {0};
14057+
bool alu32 = (BPF_CLASS(insn->code) != BPF_ALU64);
1404514058
u8 opcode = BPF_OP(insn->code);
1404614059
int err;
1404714060

@@ -14064,11 +14077,7 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env,
1406414077

1406514078
if (dst_reg->type != SCALAR_VALUE)
1406614079
ptr_reg = dst_reg;
14067-
else
14068-
/* Make sure ID is cleared otherwise dst_reg min/max could be
14069-
* incorrectly propagated into other registers by find_equal_scalars()
14070-
*/
14071-
dst_reg->id = 0;
14080+
1407214081
if (BPF_SRC(insn->code) == BPF_X) {
1407314082
src_reg = &regs[insn->src_reg];
1407414083
if (src_reg->type != SCALAR_VALUE) {
@@ -14132,7 +14141,43 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env,
1413214141
verbose(env, "verifier internal error: no src_reg\n");
1413314142
return -EINVAL;
1413414143
}
14135-
return adjust_scalar_min_max_vals(env, insn, dst_reg, *src_reg);
14144+
err = adjust_scalar_min_max_vals(env, insn, dst_reg, *src_reg);
14145+
if (err)
14146+
return err;
14147+
/*
14148+
* Compilers can generate the code
14149+
* r1 = r2
14150+
* r1 += 0x1
14151+
* if r2 < 1000 goto ...
14152+
* use r1 in memory access
14153+
* So remember constant delta between r2 and r1 and update r1 after
14154+
* 'if' condition.
14155+
*/
14156+
if (env->bpf_capable && BPF_OP(insn->code) == BPF_ADD &&
14157+
dst_reg->id && is_reg_const(src_reg, alu32)) {
14158+
u64 val = reg_const_value(src_reg, alu32);
14159+
14160+
if ((dst_reg->id & BPF_ADD_CONST) ||
14161+
/* prevent overflow in find_equal_scalars() later */
14162+
val > (u32)S32_MAX) {
14163+
/*
14164+
* If the register already went through rX += val
14165+
* we cannot accumulate another val into rx->off.
14166+
*/
14167+
dst_reg->off = 0;
14168+
dst_reg->id = 0;
14169+
} else {
14170+
dst_reg->id |= BPF_ADD_CONST;
14171+
dst_reg->off = val;
14172+
}
14173+
} else {
14174+
/*
14175+
* Make sure ID is cleared otherwise dst_reg min/max could be
14176+
* incorrectly propagated into other registers by find_equal_scalars()
14177+
*/
14178+
dst_reg->id = 0;
14179+
}
14180+
return 0;
1413614181
}
1413714182

1413814183
/* check validity of 32-bit and 64-bit arithmetic operations */
@@ -15104,12 +15149,36 @@ static bool try_match_pkt_pointers(const struct bpf_insn *insn,
1510415149
static void find_equal_scalars(struct bpf_verifier_state *vstate,
1510515150
struct bpf_reg_state *known_reg)
1510615151
{
15152+
struct bpf_reg_state fake_reg;
1510715153
struct bpf_func_state *state;
1510815154
struct bpf_reg_state *reg;
1510915155

1511015156
bpf_for_each_reg_in_vstate(vstate, state, reg, ({
15111-
if (reg->type == SCALAR_VALUE && reg->id == known_reg->id)
15157+
if (reg->type != SCALAR_VALUE || reg == known_reg)
15158+
continue;
15159+
if ((reg->id & ~BPF_ADD_CONST) != (known_reg->id & ~BPF_ADD_CONST))
15160+
continue;
15161+
if ((!(reg->id & BPF_ADD_CONST) && !(known_reg->id & BPF_ADD_CONST)) ||
15162+
reg->off == known_reg->off) {
1511215163
copy_register_state(reg, known_reg);
15164+
} else {
15165+
s32 saved_off = reg->off;
15166+
15167+
fake_reg.type = SCALAR_VALUE;
15168+
__mark_reg_known(&fake_reg, (s32)reg->off - (s32)known_reg->off);
15169+
15170+
/* reg = known_reg; reg += delta */
15171+
copy_register_state(reg, known_reg);
15172+
/*
15173+
* Must preserve off, id and add_const flag,
15174+
* otherwise another find_equal_scalars() will be incorrect.
15175+
*/
15176+
reg->off = saved_off;
15177+
15178+
scalar32_min_max_add(reg, &fake_reg);
15179+
scalar_min_max_add(reg, &fake_reg);
15180+
reg->var_off = tnum_add(reg->var_off, fake_reg.var_off);
15181+
}
1511315182
}));
1511415183
}
1511515184

@@ -16738,6 +16807,10 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
1673816807
}
1673916808
if (!rold->precise && exact == NOT_EXACT)
1674016809
return true;
16810+
if ((rold->id & BPF_ADD_CONST) != (rcur->id & BPF_ADD_CONST))
16811+
return false;
16812+
if ((rold->id & BPF_ADD_CONST) && (rold->off != rcur->off))
16813+
return false;
1674116814
/* Why check_ids() for scalar registers?
1674216815
*
1674316816
* Consider the following BPF code:

tools/testing/selftests/bpf/verifier/precise.c

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,12 @@
3939
.result = VERBOSE_ACCEPT,
4040
.errstr =
4141
"mark_precise: frame0: last_idx 26 first_idx 20\
42-
mark_precise: frame0: regs=r2 stack= before 25\
43-
mark_precise: frame0: regs=r2 stack= before 24\
44-
mark_precise: frame0: regs=r2 stack= before 23\
45-
mark_precise: frame0: regs=r2 stack= before 22\
46-
mark_precise: frame0: regs=r2 stack= before 20\
47-
mark_precise: frame0: parent state regs=r2 stack=:\
42+
mark_precise: frame0: regs=r2,r9 stack= before 25\
43+
mark_precise: frame0: regs=r2,r9 stack= before 24\
44+
mark_precise: frame0: regs=r2,r9 stack= before 23\
45+
mark_precise: frame0: regs=r2,r9 stack= before 22\
46+
mark_precise: frame0: regs=r2,r9 stack= before 20\
47+
mark_precise: frame0: parent state regs=r2,r9 stack=:\
4848
mark_precise: frame0: last_idx 19 first_idx 10\
4949
mark_precise: frame0: regs=r2,r9 stack= before 19\
5050
mark_precise: frame0: regs=r9 stack= before 18\
@@ -100,11 +100,11 @@
100100
.errstr =
101101
"26: (85) call bpf_probe_read_kernel#113\
102102
mark_precise: frame0: last_idx 26 first_idx 22\
103-
mark_precise: frame0: regs=r2 stack= before 25\
104-
mark_precise: frame0: regs=r2 stack= before 24\
105-
mark_precise: frame0: regs=r2 stack= before 23\
106-
mark_precise: frame0: regs=r2 stack= before 22\
107-
mark_precise: frame0: parent state regs=r2 stack=:\
103+
mark_precise: frame0: regs=r2,r9 stack= before 25\
104+
mark_precise: frame0: regs=r2,r9 stack= before 24\
105+
mark_precise: frame0: regs=r2,r9 stack= before 23\
106+
mark_precise: frame0: regs=r2,r9 stack= before 22\
107+
mark_precise: frame0: parent state regs=r2,r9 stack=:\
108108
mark_precise: frame0: last_idx 20 first_idx 20\
109109
mark_precise: frame0: regs=r2,r9 stack= before 20\
110110
mark_precise: frame0: parent state regs=r2,r9 stack=:\

0 commit comments

Comments
 (0)