Skip to content

Commit 4f81c16

Browse files
Alexei Starovoitovanakryiko
authored andcommitted
bpf: Recognize that two registers are safe when their ranges match
When open code iterators, bpf_loop or may_goto are used the following two states are equivalent and safe to prune the search: cur state: fp-8_w=scalar(id=3,smin=umin=smin32=umin32=2,smax=umax=smax32=umax32=11,var_off=(0x0; 0xf)) old state: fp-8_rw=scalar(id=2,smin=umin=smin32=umin32=1,smax=umax=smax32=umax32=11,var_off=(0x0; 0xf)) In other words "exact" state match should ignore liveness and precision marks, since open coded iterator logic didn't complete their propagation, reg_old->type == NOT_INIT && reg_cur->type != NOT_INIT is also not safe to prune while looping, but range_within logic that applies to scalars, ptr_to_mem, map_value, pkt_ptr is safe to rely on. Avoid doing such comparison when regular infinite loop detection logic is used, otherwise bounded loop logic will declare such "infinite loop" as false positive. Such example is in progs/verifier_loops1.c not_an_inifinite_loop(). Signed-off-by: Alexei Starovoitov <ast@kernel.org> Signed-off-by: Andrii Nakryiko <andrii@kernel.org> Acked-by: Eduard Zingerman <eddyz87@gmail.com> Acked-by: John Fastabend <john.fastabend@gmail.com> Tested-by: John Fastabend <john.fastabend@gmail.com> Link: https://lore.kernel.org/bpf/20240306031929.42666-3-alexei.starovoitov@gmail.com
1 parent 011832b commit 4f81c16

1 file changed

Lines changed: 30 additions & 21 deletions

File tree

kernel/bpf/verifier.c

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16260,8 +16260,8 @@ static int check_btf_info(struct bpf_verifier_env *env,
1626016260
}
1626116261

1626216262
/* check %cur's range satisfies %old's */
16263-
static bool range_within(struct bpf_reg_state *old,
16264-
struct bpf_reg_state *cur)
16263+
static bool range_within(const struct bpf_reg_state *old,
16264+
const struct bpf_reg_state *cur)
1626516265
{
1626616266
return old->umin_value <= cur->umin_value &&
1626716267
old->umax_value >= cur->umax_value &&
@@ -16425,21 +16425,28 @@ static bool regs_exact(const struct bpf_reg_state *rold,
1642516425
check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap);
1642616426
}
1642716427

16428+
enum exact_level {
16429+
NOT_EXACT,
16430+
EXACT,
16431+
RANGE_WITHIN
16432+
};
16433+
1642816434
/* Returns true if (rold safe implies rcur safe) */
1642916435
static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
16430-
struct bpf_reg_state *rcur, struct bpf_idmap *idmap, bool exact)
16436+
struct bpf_reg_state *rcur, struct bpf_idmap *idmap,
16437+
enum exact_level exact)
1643116438
{
16432-
if (exact)
16439+
if (exact == EXACT)
1643316440
return regs_exact(rold, rcur, idmap);
1643416441

16435-
if (!(rold->live & REG_LIVE_READ))
16442+
if (!(rold->live & REG_LIVE_READ) && exact == NOT_EXACT)
1643616443
/* explored state didn't use this */
1643716444
return true;
16438-
if (rold->type == NOT_INIT)
16439-
/* explored state can't have used this */
16440-
return true;
16441-
if (rcur->type == NOT_INIT)
16442-
return false;
16445+
if (rold->type == NOT_INIT) {
16446+
if (exact == NOT_EXACT || rcur->type == NOT_INIT)
16447+
/* explored state can't have used this */
16448+
return true;
16449+
}
1644316450

1644416451
/* Enforce that register types have to match exactly, including their
1644516452
* modifiers (like PTR_MAYBE_NULL, MEM_RDONLY, etc), as a general
@@ -16474,7 +16481,7 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
1647416481
return memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)) == 0 &&
1647516482
check_scalar_ids(rold->id, rcur->id, idmap);
1647616483
}
16477-
if (!rold->precise)
16484+
if (!rold->precise && exact == NOT_EXACT)
1647816485
return true;
1647916486
/* Why check_ids() for scalar registers?
1648016487
*
@@ -16585,7 +16592,8 @@ static struct bpf_reg_state *scalar_reg_for_stack(struct bpf_verifier_env *env,
1658516592
}
1658616593

1658716594
static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old,
16588-
struct bpf_func_state *cur, struct bpf_idmap *idmap, bool exact)
16595+
struct bpf_func_state *cur, struct bpf_idmap *idmap,
16596+
enum exact_level exact)
1658916597
{
1659016598
int i, spi;
1659116599

@@ -16598,12 +16606,13 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old,
1659816606

1659916607
spi = i / BPF_REG_SIZE;
1660016608

16601-
if (exact &&
16609+
if (exact != NOT_EXACT &&
1660216610
old->stack[spi].slot_type[i % BPF_REG_SIZE] !=
1660316611
cur->stack[spi].slot_type[i % BPF_REG_SIZE])
1660416612
return false;
1660516613

16606-
if (!(old->stack[spi].spilled_ptr.live & REG_LIVE_READ) && !exact) {
16614+
if (!(old->stack[spi].spilled_ptr.live & REG_LIVE_READ)
16615+
&& exact == NOT_EXACT) {
1660716616
i += BPF_REG_SIZE - 1;
1660816617
/* explored state didn't use this */
1660916618
continue;
@@ -16749,7 +16758,7 @@ static bool refsafe(struct bpf_func_state *old, struct bpf_func_state *cur,
1674916758
* the current state will reach 'bpf_exit' instruction safely
1675016759
*/
1675116760
static bool func_states_equal(struct bpf_verifier_env *env, struct bpf_func_state *old,
16752-
struct bpf_func_state *cur, bool exact)
16761+
struct bpf_func_state *cur, enum exact_level exact)
1675316762
{
1675416763
int i;
1675516764

@@ -16776,7 +16785,7 @@ static void reset_idmap_scratch(struct bpf_verifier_env *env)
1677616785
static bool states_equal(struct bpf_verifier_env *env,
1677716786
struct bpf_verifier_state *old,
1677816787
struct bpf_verifier_state *cur,
16779-
bool exact)
16788+
enum exact_level exact)
1678016789
{
1678116790
int i;
1678216791

@@ -17150,7 +17159,7 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
1715017159
* => unsafe memory access at 11 would not be caught.
1715117160
*/
1715217161
if (is_iter_next_insn(env, insn_idx)) {
17153-
if (states_equal(env, &sl->state, cur, true)) {
17162+
if (states_equal(env, &sl->state, cur, RANGE_WITHIN)) {
1715417163
struct bpf_func_state *cur_frame;
1715517164
struct bpf_reg_state *iter_state, *iter_reg;
1715617165
int spi;
@@ -17174,20 +17183,20 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
1717417183
goto skip_inf_loop_check;
1717517184
}
1717617185
if (is_may_goto_insn_at(env, insn_idx)) {
17177-
if (states_equal(env, &sl->state, cur, true)) {
17186+
if (states_equal(env, &sl->state, cur, RANGE_WITHIN)) {
1717817187
update_loop_entry(cur, &sl->state);
1717917188
goto hit;
1718017189
}
1718117190
goto skip_inf_loop_check;
1718217191
}
1718317192
if (calls_callback(env, insn_idx)) {
17184-
if (states_equal(env, &sl->state, cur, true))
17193+
if (states_equal(env, &sl->state, cur, RANGE_WITHIN))
1718517194
goto hit;
1718617195
goto skip_inf_loop_check;
1718717196
}
1718817197
/* attempt to detect infinite loop to avoid unnecessary doomed work */
1718917198
if (states_maybe_looping(&sl->state, cur) &&
17190-
states_equal(env, &sl->state, cur, true) &&
17199+
states_equal(env, &sl->state, cur, EXACT) &&
1719117200
!iter_active_depths_differ(&sl->state, cur) &&
1719217201
sl->state.may_goto_depth == cur->may_goto_depth &&
1719317202
sl->state.callback_unroll_depth == cur->callback_unroll_depth) {
@@ -17245,7 +17254,7 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
1724517254
*/
1724617255
loop_entry = get_loop_entry(&sl->state);
1724717256
force_exact = loop_entry && loop_entry->branches > 0;
17248-
if (states_equal(env, &sl->state, cur, force_exact)) {
17257+
if (states_equal(env, &sl->state, cur, force_exact ? RANGE_WITHIN : NOT_EXACT)) {
1724917258
if (force_exact)
1725017259
update_loop_entry(cur, loop_entry);
1725117260
hit:

0 commit comments

Comments
 (0)