Skip to content

Commit f655bad

Browse files
anakryikoAlexei Starovoitov
authored andcommitted
bpf: fix propagate_precision() logic for inner frames
Fix propagate_precision() logic to perform propagation of all necessary registers and stack slots across all active frames *in one batch step*. Doing this for each register/slot in each individual frame is wasteful, but the main problem is that backtracking of instruction in any frame except the deepest one just doesn't work. This is due to backtracking logic relying on jump history, and available jump history always starts (or ends, depending how you view it) in current frame. So, if prog A (frame #0) called subprog B (frame #1) and we need to propagate precision of, say, register R6 (callee-saved) within frame #0, we actually don't even know where jump history that corresponds to prog A even starts. We'd need to skip subprog part of jump history first to be able to do this. Luckily, with struct backtrack_state and __mark_chain_precision() handling bitmasks tracking/propagation across all active frames at the same time (added in previous patch), propagate_precision() can be both fixed and sped up by setting all the necessary bits across all frames and then performing one __mark_chain_precision() pass. This makes it unnecessary to skip subprog parts of jump history. We also improve logging along the way, to clearly specify which registers' and slots' precision markings are propagated within which frame. Each frame will have dedicated line and all registers and stack slots from that frame will be reported in format similar to precision backtrack regs/stack logging. E.g.: frame 1: propagating r1,r2,r3,fp-8,fp-16 frame 0: propagating r3,r9,fp-120 Fixes: 529409e ("bpf: propagate precision across all frames, not just the last one") Signed-off-by: Andrii Nakryiko <andrii@kernel.org> Link: https://lore.kernel.org/r/20230505043317.3629845-7-andrii@kernel.org Signed-off-by: Alexei Starovoitov <ast@kernel.org>
1 parent 1ef22b6 commit f655bad

1 file changed

Lines changed: 35 additions & 30 deletions

File tree

kernel/bpf/verifier.c

Lines changed: 35 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3726,8 +3726,7 @@ static void mark_all_scalars_imprecise(struct bpf_verifier_env *env, struct bpf_
37263726
* mark_all_scalars_imprecise() to hopefully get more permissive and generic
37273727
* finalized states which help in short circuiting more future states.
37283728
*/
3729-
static int __mark_chain_precision(struct bpf_verifier_env *env, int frame, int regno,
3730-
int spi)
3729+
static int __mark_chain_precision(struct bpf_verifier_env *env, int regno)
37313730
{
37323731
struct backtrack_state *bt = &env->bt;
37333732
struct bpf_verifier_state *st = env->cur_state;
@@ -3742,13 +3741,13 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int frame, int r
37423741
return 0;
37433742

37443743
/* set frame number from which we are starting to backtrack */
3745-
bt_init(bt, frame);
3744+
bt_init(bt, env->cur_state->curframe);
37463745

37473746
/* Do sanity checks against current state of register and/or stack
37483747
* slot, but don't set precise flag in current state, as precision
37493748
* tracking in the current state is unnecessary.
37503749
*/
3751-
func = st->frame[frame];
3750+
func = st->frame[bt->frame];
37523751
if (regno >= 0) {
37533752
reg = &func->regs[regno];
37543753
if (reg->type != SCALAR_VALUE) {
@@ -3758,13 +3757,6 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int frame, int r
37583757
bt_set_reg(bt, regno);
37593758
}
37603759

3761-
while (spi >= 0) {
3762-
if (!is_spilled_scalar_reg(&func->stack[spi]))
3763-
break;
3764-
bt_set_slot(bt, spi);
3765-
break;
3766-
}
3767-
37683760
if (bt_empty(bt))
37693761
return 0;
37703762

@@ -3914,17 +3906,15 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int frame, int r
39143906

39153907
int mark_chain_precision(struct bpf_verifier_env *env, int regno)
39163908
{
3917-
return __mark_chain_precision(env, env->cur_state->curframe, regno, -1);
3918-
}
3919-
3920-
static int mark_chain_precision_frame(struct bpf_verifier_env *env, int frame, int regno)
3921-
{
3922-
return __mark_chain_precision(env, frame, regno, -1);
3909+
return __mark_chain_precision(env, regno);
39233910
}
39243911

3925-
static int mark_chain_precision_stack_frame(struct bpf_verifier_env *env, int frame, int spi)
3912+
/* mark_chain_precision_batch() assumes that env->bt is set in the caller to
3913+
* desired reg and stack masks across all relevant frames
3914+
*/
3915+
static int mark_chain_precision_batch(struct bpf_verifier_env *env)
39263916
{
3927-
return __mark_chain_precision(env, frame, -1, spi);
3917+
return __mark_chain_precision(env, -1);
39283918
}
39293919

39303920
static bool is_spillable_regtype(enum bpf_reg_type type)
@@ -15361,20 +15351,25 @@ static int propagate_precision(struct bpf_verifier_env *env,
1536115351
struct bpf_reg_state *state_reg;
1536215352
struct bpf_func_state *state;
1536315353
int i, err = 0, fr;
15354+
bool first;
1536415355

1536515356
for (fr = old->curframe; fr >= 0; fr--) {
1536615357
state = old->frame[fr];
1536715358
state_reg = state->regs;
15359+
first = true;
1536815360
for (i = 0; i < BPF_REG_FP; i++, state_reg++) {
1536915361
if (state_reg->type != SCALAR_VALUE ||
1537015362
!state_reg->precise ||
1537115363
!(state_reg->live & REG_LIVE_READ))
1537215364
continue;
15373-
if (env->log.level & BPF_LOG_LEVEL2)
15374-
verbose(env, "frame %d: propagating r%d\n", fr, i);
15375-
err = mark_chain_precision_frame(env, fr, i);
15376-
if (err < 0)
15377-
return err;
15365+
if (env->log.level & BPF_LOG_LEVEL2) {
15366+
if (first)
15367+
verbose(env, "frame %d: propagating r%d", fr, i);
15368+
else
15369+
verbose(env, ",r%d", i);
15370+
}
15371+
bt_set_frame_reg(&env->bt, fr, i);
15372+
first = false;
1537815373
}
1537915374

1538015375
for (i = 0; i < state->allocated_stack / BPF_REG_SIZE; i++) {
@@ -15385,14 +15380,24 @@ static int propagate_precision(struct bpf_verifier_env *env,
1538515380
!state_reg->precise ||
1538615381
!(state_reg->live & REG_LIVE_READ))
1538715382
continue;
15388-
if (env->log.level & BPF_LOG_LEVEL2)
15389-
verbose(env, "frame %d: propagating fp%d\n",
15390-
fr, (-i - 1) * BPF_REG_SIZE);
15391-
err = mark_chain_precision_stack_frame(env, fr, i);
15392-
if (err < 0)
15393-
return err;
15383+
if (env->log.level & BPF_LOG_LEVEL2) {
15384+
if (first)
15385+
verbose(env, "frame %d: propagating fp%d",
15386+
fr, (-i - 1) * BPF_REG_SIZE);
15387+
else
15388+
verbose(env, ",fp%d", (-i - 1) * BPF_REG_SIZE);
15389+
}
15390+
bt_set_frame_slot(&env->bt, fr, i);
15391+
first = false;
1539415392
}
15393+
if (!first)
15394+
verbose(env, "\n");
1539515395
}
15396+
15397+
err = mark_chain_precision_batch(env);
15398+
if (err < 0)
15399+
return err;
15400+
1539615401
return 0;
1539715402
}
1539815403

0 commit comments

Comments
 (0)