mirrors/linux
1
0
Fork 1
mirror of https://github.com/torvalds/linux.git synced 2025-11-04 10:40:15 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

bpf: WARN_ONCE on verifier bugs

Browse source 
Throughout the verifier's logic, there are multiple checks for
inconsistent states that should never happen and would indicate a
verifier bug. These bugs are typically logged in the verifier logs and
sometimes preceded by a WARN_ONCE.

This patch reworks these checks to consistently emit a verifier log AND
a warning when CONFIG_DEBUG_KERNEL is enabled. The consistent use of
WARN_ONCE should help fuzzers (ex. syzkaller) expose any situation
where they are actually able to reach one of those buggy verifier
states.

Acked-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
Link: https://lore.kernel.org/r/aCs1nYvNNMq8dAWP@mail.gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
This commit is contained in:
Paul Chaignon 2025-05-19 15:43:57 +02:00 committed by Alexei Starovoitov
parent 9325d53fe9
commit 1cb0f56d96
4 changed files with 79 additions and 83 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

6
include/linux/bpf.h
View file

@ -346,6 +346,12 @@ static inline const char *btf_field_type_name(enum btf_field_type type)
} }
} }
#if IS_ENABLED(CONFIG_DEBUG_KERNEL)
#define BPF_WARN_ONCE(cond, format...) WARN_ONCE(cond, format)
#else
#define BPF_WARN_ONCE(cond, format...) BUILD_BUG_ON_INVALID(cond)
#endif
static inline u32 btf_field_type_size(enum btf_field_type type) static inline u32 btf_field_type_size(enum btf_field_type type)
{ {
switch (type) { switch (type) {

11
include/linux/bpf_verifier.h
View file

@ -839,6 +839,17 @@ __printf(3, 4) void verbose_linfo(struct bpf_verifier_env *env,
u32 insn_off, u32 insn_off,
const char *prefix_fmt, ...); const char *prefix_fmt, ...);
#define verifier_bug_if(cond, env, fmt, args...) \
({ \
bool __cond = (cond); \
if (unlikely(__cond)) { \
BPF_WARN_ONCE(1, "verifier bug: " fmt "(" #cond ")\n", ##args); \
bpf_log(&env->log, "verifier bug: " fmt "(" #cond ")\n", ##args); \
} \
(__cond); \
})
#define verifier_bug(env, fmt, args...) verifier_bug_if(1, env, fmt, ##args)
static inline struct bpf_func_state *cur_func(struct bpf_verifier_env *env) static inline struct bpf_func_state *cur_func(struct bpf_verifier_env *env)
{ {
struct bpf_verifier_state *cur = env->cur_state; struct bpf_verifier_state *cur = env->cur_state;

4
kernel/bpf/btf.c
View file

@ -7659,7 +7659,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
return 0; return 0;
if (!prog->aux->func_info) { if (!prog->aux->func_info) {
bpf_log(log, "Verifier bug\n"); verifier_bug(env, "func_info undefined");
return -EFAULT; return -EFAULT;
} }
@ -7683,7 +7683,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
tname = btf_name_by_offset(btf, fn_t->name_off); tname = btf_name_by_offset(btf, fn_t->name_off);
if (prog->aux->func_info_aux[subprog].unreliable) { if (prog->aux->func_info_aux[subprog].unreliable) {
bpf_log(log, "Verifier bug in function %s()\n", tname); verifier_bug(env, "unreliable BTF for function %s()", tname);
return -EFAULT; return -EFAULT;
} }
if (prog_type == BPF_PROG_TYPE_EXT) if (prog_type == BPF_PROG_TYPE_EXT)

127
kernel/bpf/verifier.c
View file

@ -1924,11 +1924,8 @@ static struct bpf_verifier_state *get_loop_entry(struct bpf_verifier_env *env,
u32 steps = 0; u32 steps = 0;
while (topmost && topmost->loop_entry) { while (topmost && topmost->loop_entry) {
if (steps++ > st->dfs_depth) { if (verifier_bug_if(steps++ > st->dfs_depth, env, "infinite loop"))
WARN_ONCE(true, "verifier bug: infinite loop in get_loop_entry\n");
verbose(env, "verifier bug: infinite loop in get_loop_entry()\n");
return ERR_PTR(-EFAULT); return ERR_PTR(-EFAULT);
}
topmost = topmost->loop_entry; topmost = topmost->loop_entry;
} }
return topmost; return topmost;
@ -3460,12 +3457,11 @@ static int mark_reg_read(struct bpf_verifier_env *env,
/* if read wasn't screened by an earlier write ... */ /* if read wasn't screened by an earlier write ... */
if (writes && state->live & REG_LIVE_WRITTEN) if (writes && state->live & REG_LIVE_WRITTEN)
break; break;
if (parent->live & REG_LIVE_DONE) { if (verifier_bug_if(parent->live & REG_LIVE_DONE, env,
verbose(env, "verifier BUG type %s var_off %lld off %d\n", "type %s var_off %lld off %d",
reg_type_str(env, parent->type), reg_type_str(env, parent->type),
parent->var_off.value, parent->off); parent->var_off.value, parent->off))
return -EFAULT; return -EFAULT;
}
/* The first condition is more likely to be true than the /* The first condition is more likely to be true than the
* second, checked it first. * second, checked it first.
*/ */
@ -3858,13 +3854,13 @@ static int push_insn_history(struct bpf_verifier_env *env, struct bpf_verifier_s
/* atomic instructions push insn_flags twice, for READ and /* atomic instructions push insn_flags twice, for READ and
* WRITE sides, but they should agree on stack slot * WRITE sides, but they should agree on stack slot
*/ */
WARN_ONCE((env->cur_hist_ent->flags & insn_flags) && verifier_bug_if((env->cur_hist_ent->flags & insn_flags) &&
(env->cur_hist_ent->flags & insn_flags) != insn_flags, (env->cur_hist_ent->flags & insn_flags) != insn_flags,
"verifier insn history bug: insn_idx %d cur flags %x new flags %x\n", env, "insn history: insn_idx %d cur flags %x new flags %x",
env->insn_idx, env->cur_hist_ent->flags, insn_flags); env->insn_idx, env->cur_hist_ent->flags, insn_flags);
env->cur_hist_ent->flags |= insn_flags; env->cur_hist_ent->flags |= insn_flags;
WARN_ONCE(env->cur_hist_ent->linked_regs != 0, verifier_bug_if(env->cur_hist_ent->linked_regs != 0, env,
"verifier insn history bug: insn_idx %d linked_regs != 0: %#llx\n", "insn history: insn_idx %d linked_regs: %#llx",
env->insn_idx, env->cur_hist_ent->linked_regs); env->insn_idx, env->cur_hist_ent->linked_regs);
env->cur_hist_ent->linked_regs = linked_regs; env->cur_hist_ent->linked_regs = linked_regs;
return 0; return 0;
@ -3988,8 +3984,7 @@ static inline u32 bt_empty(struct backtrack_state *bt)
static inline int bt_subprog_enter(struct backtrack_state *bt) static inline int bt_subprog_enter(struct backtrack_state *bt)
{ {
if (bt->frame == MAX_CALL_FRAMES - 1) { if (bt->frame == MAX_CALL_FRAMES - 1) {
verbose(bt->env, "BUG subprog enter from frame %d\n", bt->frame); verifier_bug(bt->env, "subprog enter from frame %d", bt->frame);
WARN_ONCE(1, "verifier backtracking bug");
return -EFAULT; return -EFAULT;
} }
bt->frame++; bt->frame++;
@ -3999,8 +3994,7 @@ static inline int bt_subprog_enter(struct backtrack_state *bt)
static inline int bt_subprog_exit(struct backtrack_state *bt) static inline int bt_subprog_exit(struct backtrack_state *bt)
{ {
if (bt->frame == 0) { if (bt->frame == 0) {
verbose(bt->env, "BUG subprog exit from frame 0\n"); verifier_bug(bt->env, "subprog exit from frame 0");
WARN_ONCE(1, "verifier backtracking bug");
return -EFAULT; return -EFAULT;
} }
bt->frame--; bt->frame--;
@ -4278,14 +4272,15 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
* should be literally next instruction in * should be literally next instruction in
* caller program * caller program
*/ */
WARN_ONCE(idx + 1 != subseq_idx, "verifier backtracking bug"); verifier_bug_if(idx + 1 != subseq_idx, env,
"extra insn from subprog");
/* r1-r5 are invalidated after subprog call, /* r1-r5 are invalidated after subprog call,
* so for global func call it shouldn't be set * so for global func call it shouldn't be set
* anymore * anymore
*/ */
if (bt_reg_mask(bt) & BPF_REGMASK_ARGS) { if (bt_reg_mask(bt) & BPF_REGMASK_ARGS) {
verbose(env, "BUG regs %x\n", bt_reg_mask(bt)); verifier_bug(env, "global subprog unexpected regs %x",
WARN_ONCE(1, "verifier backtracking bug"); bt_reg_mask(bt));
return -EFAULT; return -EFAULT;
} }
/* global subprog always sets R0 */ /* global subprog always sets R0 */
@ -4299,16 +4294,17 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
* the current frame should be zero by now * the current frame should be zero by now
*/ */
if (bt_reg_mask(bt) & ~BPF_REGMASK_ARGS) { if (bt_reg_mask(bt) & ~BPF_REGMASK_ARGS) {
verbose(env, "BUG regs %x\n", bt_reg_mask(bt)); verifier_bug(env, "static subprog unexpected regs %x",
WARN_ONCE(1, "verifier backtracking bug"); bt_reg_mask(bt));
return -EFAULT; return -EFAULT;
} }
/* we are now tracking register spills correctly, /* we are now tracking register spills correctly,
* so any instance of leftover slots is a bug * so any instance of leftover slots is a bug
*/ */
if (bt_stack_mask(bt) != 0) { if (bt_stack_mask(bt) != 0) {
verbose(env, "BUG stack slots %llx\n", bt_stack_mask(bt)); verifier_bug(env,
WARN_ONCE(1, "verifier backtracking bug (subprog leftover stack slots)"); "static subprog leftover stack slots %llx",
bt_stack_mask(bt));
return -EFAULT; return -EFAULT;
} }
/* propagate r1-r5 to the caller */ /* propagate r1-r5 to the caller */
@ -4331,13 +4327,13 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
* not actually arguments passed directly to callback subprogs * not actually arguments passed directly to callback subprogs
*/ */
if (bt_reg_mask(bt) & ~BPF_REGMASK_ARGS) { if (bt_reg_mask(bt) & ~BPF_REGMASK_ARGS) {
verbose(env, "BUG regs %x\n", bt_reg_mask(bt)); verifier_bug(env, "callback unexpected regs %x",
WARN_ONCE(1, "verifier backtracking bug"); bt_reg_mask(bt));
return -EFAULT; return -EFAULT;
} }
if (bt_stack_mask(bt) != 0) { if (bt_stack_mask(bt) != 0) {
verbose(env, "BUG stack slots %llx\n", bt_stack_mask(bt)); verifier_bug(env, "callback leftover stack slots %llx",
WARN_ONCE(1, "verifier backtracking bug (callback leftover stack slots)"); bt_stack_mask(bt));
return -EFAULT; return -EFAULT;
} }
/* clear r1-r5 in callback subprog's mask */ /* clear r1-r5 in callback subprog's mask */
@ -4356,11 +4352,11 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
/* regular helper call sets R0 */ /* regular helper call sets R0 */
bt_clear_reg(bt, BPF_REG_0); bt_clear_reg(bt, BPF_REG_0);
if (bt_reg_mask(bt) & BPF_REGMASK_ARGS) { if (bt_reg_mask(bt) & BPF_REGMASK_ARGS) {
/* if backtracing was looking for registers R1-R5 /* if backtracking was looking for registers R1-R5
* they should have been found already. * they should have been found already.
*/ */
verbose(env, "BUG regs %x\n", bt_reg_mask(bt)); verifier_bug(env, "backtracking call unexpected regs %x",
WARN_ONCE(1, "verifier backtracking bug"); bt_reg_mask(bt));
return -EFAULT; return -EFAULT;
} }
} else if (opcode == BPF_EXIT) { } else if (opcode == BPF_EXIT) {
@ -4378,8 +4374,8 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
for (i = BPF_REG_1; i <= BPF_REG_5; i++) for (i = BPF_REG_1; i <= BPF_REG_5; i++)
bt_clear_reg(bt, i); bt_clear_reg(bt, i);
if (bt_reg_mask(bt) & BPF_REGMASK_ARGS) { if (bt_reg_mask(bt) & BPF_REGMASK_ARGS) {
verbose(env, "BUG regs %x\n", bt_reg_mask(bt)); verifier_bug(env, "backtracking exit unexpected regs %x",
WARN_ONCE(1, "verifier backtracking bug"); bt_reg_mask(bt));
return -EFAULT; return -EFAULT;
} }
@ -4720,9 +4716,8 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int regno)
return 0; return 0;
} }
verbose(env, "BUG backtracking func entry subprog %d reg_mask %x stack_mask %llx\n", verifier_bug(env, "backtracking func entry subprog %d reg_mask %x stack_mask %llx",
st->frame[0]->subprogno, bt_reg_mask(bt), bt_stack_mask(bt)); st->frame[0]->subprogno, bt_reg_mask(bt), bt_stack_mask(bt));
WARN_ONCE(1, "verifier backtracking bug");
return -EFAULT; return -EFAULT;
} }
@ -4758,8 +4753,7 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int regno)
* It means the backtracking missed the spot where * It means the backtracking missed the spot where
* particular register was initialized with a constant. * particular register was initialized with a constant.
*/ */
verbose(env, "BUG backtracking idx %d\n", i); verifier_bug(env, "backtracking idx %d", i);
WARN_ONCE(1, "verifier backtracking bug");
return -EFAULT; return -EFAULT;
} }
} }
@ -4784,12 +4778,10 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int regno)
bitmap_from_u64(mask, bt_frame_stack_mask(bt, fr)); bitmap_from_u64(mask, bt_frame_stack_mask(bt, fr));
for_each_set_bit(i, mask, 64) { for_each_set_bit(i, mask, 64) {
if (i >= func->allocated_stack / BPF_REG_SIZE) { if (verifier_bug_if(i >= func->allocated_stack / BPF_REG_SIZE,
verbose(env, "BUG backtracking (stack slot %d, total slots %d)\n", env, "stack slot %d, total slots %d",
i, func->allocated_stack / BPF_REG_SIZE); i, func->allocated_stack / BPF_REG_SIZE))
WARN_ONCE(1, "verifier backtracking bug (stack slot out of bounds)");
return -EFAULT; return -EFAULT;
}
if (!is_spilled_scalar_reg(&func->stack[i])) { if (!is_spilled_scalar_reg(&func->stack[i])) {
bt_clear_frame_slot(bt, fr, i); bt_clear_frame_slot(bt, fr, i);
@ -6562,21 +6554,18 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx,
/* find the callee */ /* find the callee */
next_insn = i + insn[i].imm + 1; next_insn = i + insn[i].imm + 1;
sidx = find_subprog(env, next_insn); sidx = find_subprog(env, next_insn);
if (sidx < 0) { if (verifier_bug_if(sidx < 0, env, "callee not found at insn %d", next_insn))
WARN_ONCE(1, "verifier bug. No program starts at insn %d\n",
next_insn);
return -EFAULT; return -EFAULT;
}
if (subprog[sidx].is_async_cb) { if (subprog[sidx].is_async_cb) {
if (subprog[sidx].has_tail_call) { if (subprog[sidx].has_tail_call) {
verbose(env, "verifier bug. subprog has tail_call and async cb\n"); verifier_bug(env, "subprog has tail_call and async cb");
return -EFAULT; return -EFAULT;
} }
/* async callbacks don't increase bpf prog stack size unless called directly */ /* async callbacks don't increase bpf prog stack size unless called directly */
if (!bpf_pseudo_call(insn + i)) if (!bpf_pseudo_call(insn + i))
continue; continue;
if (subprog[sidx].is_exception_cb) { if (subprog[sidx].is_exception_cb) {
verbose(env, "insn %d cannot call exception cb directly\n", i); verbose(env, "insn %d cannot call exception cb directly", i);
return -EINVAL; return -EINVAL;
} }
} }
@ -6676,11 +6665,8 @@ static int get_callee_stack_depth(struct bpf_verifier_env *env,
int start = idx + insn->imm + 1, subprog; int start = idx + insn->imm + 1, subprog;
subprog = find_subprog(env, start); subprog = find_subprog(env, start);
if (subprog < 0) { if (verifier_bug_if(subprog < 0, env, "get stack depth: no program at insn %d", start))
WARN_ONCE(1, "verifier bug. No program starts at insn %d\n",
start);
return -EFAULT; return -EFAULT;
}
return env->subprog_info[subprog].stack_depth; return env->subprog_info[subprog].stack_depth;
} }
#endif #endif
@ -7985,7 +7971,7 @@ static int check_stack_range_initialized(
slot = -i - 1; slot = -i - 1;
spi = slot / BPF_REG_SIZE; spi = slot / BPF_REG_SIZE;
if (state->allocated_stack <= slot) { if (state->allocated_stack <= slot) {
verbose(env, "verifier bug: allocated_stack too small\n"); verbose(env, "allocated_stack too small\n");
return -EFAULT; return -EFAULT;
} }
@ -8414,7 +8400,7 @@ static int process_timer_func(struct bpf_verifier_env *env, int regno,
return -EINVAL; return -EINVAL;
} }
if (meta->map_ptr) { if (meta->map_ptr) {
verbose(env, "verifier bug. Two map pointers in a timer helper\n"); verifier_bug(env, "Two map pointers in a timer helper");
return -EFAULT; return -EFAULT;
} }
meta->map_uid = reg->map_uid; meta->map_uid = reg->map_uid;
@ -10286,8 +10272,7 @@ static int setup_func_entry(struct bpf_verifier_env *env, int subprog, int calls
} }
if (state->frame[state->curframe + 1]) { if (state->frame[state->curframe + 1]) {
verbose(env, "verifier bug. Frame %d already allocated\n", verifier_bug(env, "Frame %d already allocated", state->curframe + 1);
state->curframe + 1);
return -EFAULT; return -EFAULT;
} }
@ -10401,8 +10386,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
if (err) if (err)
return err; return err;
} else { } else {
bpf_log(log, "verifier bug: unrecognized arg#%d type %d\n", verifier_bug(env, "unrecognized arg#%d type %d", i, arg->arg_type);
i, arg->arg_type);
return -EFAULT; return -EFAULT;
} }
} }
@ -10465,12 +10449,12 @@ static int push_callback_call(struct bpf_verifier_env *env, struct bpf_insn *ins
env->subprog_info[subprog].is_cb = true; env->subprog_info[subprog].is_cb = true;
if (bpf_pseudo_kfunc_call(insn) && if (bpf_pseudo_kfunc_call(insn) &&
!is_callback_calling_kfunc(insn->imm)) { !is_callback_calling_kfunc(insn->imm)) {
verbose(env, "verifier bug: kfunc %s#%d not marked as callback-calling\n", verifier_bug(env, "kfunc %s#%d not marked as callback-calling",
func_id_name(insn->imm), insn->imm); func_id_name(insn->imm), insn->imm);
return -EFAULT; return -EFAULT;
} else if (!bpf_pseudo_kfunc_call(insn) && } else if (!bpf_pseudo_kfunc_call(insn) &&
!is_callback_calling_function(insn->imm)) { /* helper */ !is_callback_calling_function(insn->imm)) { /* helper */
verbose(env, "verifier bug: helper %s#%d not marked as callback-calling\n", verifier_bug(env, "helper %s#%d not marked as callback-calling",
func_id_name(insn->imm), insn->imm); func_id_name(insn->imm), insn->imm);
return -EFAULT; return -EFAULT;
} }
@ -10523,10 +10507,9 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
target_insn = *insn_idx + insn->imm + 1; target_insn = *insn_idx + insn->imm + 1;
subprog = find_subprog(env, target_insn); subprog = find_subprog(env, target_insn);
if (subprog < 0) { if (verifier_bug_if(subprog < 0, env, "target of func call at insn %d is not a program",
verbose(env, "verifier bug. No program starts at insn %d\n", target_insn); target_insn))
return -EFAULT; return -EFAULT;
}
caller = state->frame[state->curframe]; caller = state->frame[state->curframe];
err = btf_check_subprog_call(env, subprog, caller->regs); err = btf_check_subprog_call(env, subprog, caller->regs);
@ -11125,7 +11108,7 @@ static int check_bpf_snprintf_call(struct bpf_verifier_env *env,
err = fmt_map->ops->map_direct_value_addr(fmt_map, &fmt_addr, err = fmt_map->ops->map_direct_value_addr(fmt_map, &fmt_addr,
fmt_map_off); fmt_map_off);
if (err) { if (err) {
verbose(env, "verifier bug\n"); verbose(env, "failed to retrieve map value address\n");
return -EFAULT; return -EFAULT;
} }
fmt = (char *)(long)fmt_addr + fmt_map_off; fmt = (char *)(long)fmt_addr + fmt_map_off;
@ -19706,10 +19689,9 @@ static int do_check(struct bpf_verifier_env *env)
return err; return err;
break; break;
} else { } else {
if (WARN_ON_ONCE(env->cur_state->loop_entry)) { if (verifier_bug_if(env->cur_state->loop_entry, env,
verbose(env, "verifier bug: env->cur_state->loop_entry != NULL\n"); "broken loop detection"))
return -EFAULT; return -EFAULT;
}
do_print_state = true; do_print_state = true;
continue; continue;
} }
@ -20767,10 +20749,9 @@ static int opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env,
if (bpf_pseudo_kfunc_call(&insn)) if (bpf_pseudo_kfunc_call(&insn))
continue; continue;
if (WARN_ON(load_reg == -1)) { if (verifier_bug_if(load_reg == -1, env,
verbose(env, "verifier bug. zext_dst is set, but no reg is defined\n"); "zext_dst is set, but no reg is defined"))
return -EFAULT; return -EFAULT;
}
zext_patch[0] = insn; zext_patch[0] = insn;
zext_patch[1].dst_reg = load_reg; zext_patch[1].dst_reg = load_reg;
@ -21087,11 +21068,9 @@ static int jit_subprogs(struct bpf_verifier_env *env)
* propagated in any case. * propagated in any case.
*/ */
subprog = find_subprog(env, i + insn->imm + 1); subprog = find_subprog(env, i + insn->imm + 1);
if (subprog < 0) { if (verifier_bug_if(subprog < 0, env, "No program to jit at insn %d",
WARN_ONCE(1, "verifier bug. No program starts at insn %d\n", i + insn->imm + 1))
i + insn->imm + 1);
return -EFAULT; return -EFAULT;
}
/* temporarily remember subprog id inside insn instead of /* temporarily remember subprog id inside insn instead of
* aux_data, since next loop will split up all insns into funcs * aux_data, since next loop will split up all insns into funcs
*/ */
@ -22454,7 +22433,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
continue; continue;
/* We need two slots in case timed may_goto is supported. */ /* We need two slots in case timed may_goto is supported. */
if (stack_slots > slots) { if (stack_slots > slots) {
verbose(env, "verifier bug: stack_slots supports may_goto only\n"); verifier_bug(env, "stack_slots supports may_goto only");
return -EFAULT; return -EFAULT;
} }