* [PATCH bpf-next v4] bpf: WARN_ONCE on verifier bugs
@ 2025-05-16 9:34 Paul Chaignon
2025-05-16 16:14 ` Andrii Nakryiko
0 siblings, 1 reply; 4+ messages in thread
From: Paul Chaignon @ 2025-05-16 9:34 UTC (permalink / raw)
To: bpf; +Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
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.
Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
---
Changes in v4:
- Evaluate condition once and stringify it, as suggested by Alexei.
- Use verifier_bug_if instead of verifier_bug where it can help
disambiguate the callsite or shorten the message.
- Add newline character in verifier_bug_if directly.
Changes in v3:
- Introduce and use verifier_bug_if, as suggested by Andrii.
Changes in v2:
- Introduce a new BPF_WARN_ONCE macro, with WARN_ONCE conditioned on
CONFIG_DEBUG_KERNEL, as per reviews.
- Use the new helper function for verifier bugs missed in v1,
particularly around backtracking.
include/linux/bpf.h | 6 ++
include/linux/bpf_verifier.h | 11 +++
kernel/bpf/btf.c | 4 +-
kernel/bpf/verifier.c | 140 +++++++++++++++--------------------
4 files changed, 77 insertions(+), 84 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 83c56f40842b..5b25d278409b 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -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)
{
switch (type) {
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index cedd66867ecf..7edb15830132 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -839,6 +839,17 @@ __printf(3, 4) void verbose_linfo(struct bpf_verifier_env *env,
u32 insn_off,
const char *prefix_fmt, ...);
+#define verifier_bug_if(cond, env, fmt, args...) \
+ ({ \
+ bool __cond = unlikely(cond); \
+ if (__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)
{
struct bpf_verifier_state *cur = env->cur_state;
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 6b21ca67070c..0f7828380895 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -7659,7 +7659,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
return 0;
if (!prog->aux->func_info) {
- bpf_log(log, "Verifier bug\n");
+ verifier_bug(env, "func_info undefined");
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);
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;
}
if (prog_type == BPF_PROG_TYPE_EXT)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index f6d3655b3a7a..cec35daf2b77 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1924,11 +1924,9 @@ static struct bpf_verifier_state *get_loop_entry(struct bpf_verifier_env *env,
u32 steps = 0;
while (topmost && topmost->loop_entry) {
- if (steps++ > st->dfs_depth) {
- WARN_ONCE(true, "verifier bug: infinite loop in get_loop_entry\n");
- verbose(env, "verifier bug: infinite loop in get_loop_entry()\n");
+ if (verifier_bug_if(steps++ > st->dfs_depth, env,
+ "infinite loop"))
return ERR_PTR(-EFAULT);
- }
topmost = topmost->loop_entry;
}
return topmost;
@@ -3460,12 +3458,11 @@ static int mark_reg_read(struct bpf_verifier_env *env,
/* if read wasn't screened by an earlier write ... */
if (writes && state->live & REG_LIVE_WRITTEN)
break;
- if (parent->live & REG_LIVE_DONE) {
- verbose(env, "verifier BUG type %s var_off %lld off %d\n",
- reg_type_str(env, parent->type),
- parent->var_off.value, parent->off);
+ if (verifier_bug_if(parent->live & REG_LIVE_DONE, env,
+ "type %s var_off %lld off %d",
+ reg_type_str(env, parent->type),
+ parent->var_off.value, parent->off))
return -EFAULT;
- }
/* The first condition is more likely to be true than the
* second, checked it first.
*/
@@ -3858,14 +3855,14 @@ static int push_insn_history(struct bpf_verifier_env *env, struct bpf_verifier_s
/* atomic instructions push insn_flags twice, for READ and
* WRITE sides, but they should agree on stack slot
*/
- WARN_ONCE((env->cur_hist_ent->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_idx, 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, "insn history: insn_idx %d cur flags %x new flags %x",
+ env->insn_idx, env->cur_hist_ent->flags, insn_flags);
env->cur_hist_ent->flags |= insn_flags;
- WARN_ONCE(env->cur_hist_ent->linked_regs != 0,
- "verifier insn history bug: insn_idx %d linked_regs != 0: %#llx\n",
- env->insn_idx, env->cur_hist_ent->linked_regs);
+ verifier_bug_if(env->cur_hist_ent->linked_regs != 0, env,
+ "insn history: insn_idx %d linked_regs: %#llx",
+ env->insn_idx, env->cur_hist_ent->linked_regs);
env->cur_hist_ent->linked_regs = linked_regs;
return 0;
}
@@ -3988,8 +3985,7 @@ static inline u32 bt_empty(struct backtrack_state *bt)
static inline int bt_subprog_enter(struct backtrack_state *bt)
{
if (bt->frame == MAX_CALL_FRAMES - 1) {
- verbose(bt->env, "BUG subprog enter from frame %d\n", bt->frame);
- WARN_ONCE(1, "verifier backtracking bug");
+ verifier_bug(bt->env, "subprog enter from frame %d", bt->frame);
return -EFAULT;
}
bt->frame++;
@@ -3999,8 +3995,7 @@ static inline int bt_subprog_enter(struct backtrack_state *bt)
static inline int bt_subprog_exit(struct backtrack_state *bt)
{
if (bt->frame == 0) {
- verbose(bt->env, "BUG subprog exit from frame 0\n");
- WARN_ONCE(1, "verifier backtracking bug");
+ verifier_bug(bt->env, "subprog exit from frame 0");
return -EFAULT;
}
bt->frame--;
@@ -4278,14 +4273,15 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
* should be literally next instruction in
* 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,
* so for global func call it shouldn't be set
* anymore
*/
if (bt_reg_mask(bt) & BPF_REGMASK_ARGS) {
- verbose(env, "BUG regs %x\n", bt_reg_mask(bt));
- WARN_ONCE(1, "verifier backtracking bug");
+ verifier_bug(env, "scratch reg set: regs %x",
+ bt_reg_mask(bt));
return -EFAULT;
}
/* global subprog always sets R0 */
@@ -4299,16 +4295,16 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
* the current frame should be zero by now
*/
if (bt_reg_mask(bt) & ~BPF_REGMASK_ARGS) {
- verbose(env, "BUG regs %x\n", bt_reg_mask(bt));
- WARN_ONCE(1, "verifier backtracking bug");
+ verifier_bug(env, "unexpected precise regs %x",
+ bt_reg_mask(bt));
return -EFAULT;
}
/* we are now tracking register spills correctly,
* so any instance of leftover slots is a bug
*/
if (bt_stack_mask(bt) != 0) {
- verbose(env, "BUG stack slots %llx\n", bt_stack_mask(bt));
- WARN_ONCE(1, "verifier backtracking bug (subprog leftover stack slots)");
+ verifier_bug(env, "subprog leftover stack slots %llx",
+ bt_stack_mask(bt));
return -EFAULT;
}
/* 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
*/
if (bt_reg_mask(bt) & ~BPF_REGMASK_ARGS) {
- verbose(env, "BUG regs %x\n", bt_reg_mask(bt));
- WARN_ONCE(1, "verifier backtracking bug");
+ verifier_bug(env, "unexpected precise regs %x",
+ bt_reg_mask(bt));
return -EFAULT;
}
if (bt_stack_mask(bt) != 0) {
- verbose(env, "BUG stack slots %llx\n", bt_stack_mask(bt));
- WARN_ONCE(1, "verifier backtracking bug (callback leftover stack slots)");
+ verifier_bug(env, "callback leftover stack slots %llx",
+ bt_stack_mask(bt));
return -EFAULT;
}
/* clear r1-r5 in callback subprog's mask */
@@ -4359,8 +4355,7 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
/* if backtracing was looking for registers R1-R5
* they should have been found already.
*/
- verbose(env, "BUG regs %x\n", bt_reg_mask(bt));
- WARN_ONCE(1, "verifier backtracking bug");
+ verifier_bug(env, "regs not found %x", bt_reg_mask(bt));
return -EFAULT;
}
} else if (opcode == BPF_EXIT) {
@@ -4378,8 +4373,7 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
for (i = BPF_REG_1; i <= BPF_REG_5; i++)
bt_clear_reg(bt, i);
if (bt_reg_mask(bt) & BPF_REGMASK_ARGS) {
- verbose(env, "BUG regs %x\n", bt_reg_mask(bt));
- WARN_ONCE(1, "verifier backtracking bug");
+ verifier_bug(env, "regs not found %x", bt_reg_mask(bt));
return -EFAULT;
}
@@ -4720,9 +4714,8 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int regno)
return 0;
}
- verbose(env, "BUG backtracking func entry subprog %d reg_mask %x stack_mask %llx\n",
- st->frame[0]->subprogno, bt_reg_mask(bt), bt_stack_mask(bt));
- WARN_ONCE(1, "verifier backtracking bug");
+ 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));
return -EFAULT;
}
@@ -4751,17 +4744,14 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int regno)
i = get_prev_insn_idx(env, st, i, hist_start, &hist_end);
if (i == -ENOENT)
break;
- if (i >= env->prog->len) {
+ if (verifier_bug_if(i >= env->prog->len, env, "backtracking idx %d", i))
/* This can happen if backtracking reached insn 0
* and there are still reg_mask or stack_mask
* to backtrack.
* It means the backtracking missed the spot where
* particular register was initialized with a constant.
*/
- verbose(env, "BUG backtracking idx %d\n", i);
- WARN_ONCE(1, "verifier backtracking bug");
return -EFAULT;
- }
}
st = st->parent;
if (!st)
@@ -4784,12 +4774,10 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int regno)
bitmap_from_u64(mask, bt_frame_stack_mask(bt, fr));
for_each_set_bit(i, mask, 64) {
- if (i >= func->allocated_stack / BPF_REG_SIZE) {
- verbose(env, "BUG backtracking (stack slot %d, total slots %d)\n",
- i, func->allocated_stack / BPF_REG_SIZE);
- WARN_ONCE(1, "verifier backtracking bug (stack slot out of bounds)");
+ if (verifier_bug_if(i >= func->allocated_stack / BPF_REG_SIZE,
+ env, "stack slot %d, total slots %d",
+ i, func->allocated_stack / BPF_REG_SIZE))
return -EFAULT;
- }
if (!is_spilled_scalar_reg(&func->stack[i])) {
bt_clear_frame_slot(bt, fr, i);
@@ -6562,21 +6550,18 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx,
/* find the callee */
next_insn = i + insn[i].imm + 1;
sidx = find_subprog(env, next_insn);
- if (sidx < 0) {
- WARN_ONCE(1, "verifier bug. No program starts at insn %d\n",
- next_insn);
+ if (verifier_bug_if(sidx < 0, env, "No program starts at insn %d", next_insn))
return -EFAULT;
- }
if (subprog[sidx].is_async_cb) {
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;
}
/* async callbacks don't increase bpf prog stack size unless called directly */
if (!bpf_pseudo_call(insn + i))
continue;
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;
}
}
@@ -6676,11 +6661,8 @@ static int get_callee_stack_depth(struct bpf_verifier_env *env,
int start = idx + insn->imm + 1, subprog;
subprog = find_subprog(env, start);
- if (subprog < 0) {
- WARN_ONCE(1, "verifier bug. No program starts at insn %d\n",
- start);
+ if (verifier_bug_if(subprog < 0, env, "No program starts at insn %d", start))
return -EFAULT;
- }
return env->subprog_info[subprog].stack_depth;
}
#endif
@@ -7985,7 +7967,7 @@ static int check_stack_range_initialized(
slot = -i - 1;
spi = slot / BPF_REG_SIZE;
if (state->allocated_stack <= slot) {
- verbose(env, "verifier bug: allocated_stack too small\n");
+ verbose(env, "allocated_stack too small\n");
return -EFAULT;
}
@@ -8414,7 +8396,7 @@ static int process_timer_func(struct bpf_verifier_env *env, int regno,
return -EINVAL;
}
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;
}
meta->map_uid = reg->map_uid;
@@ -10286,8 +10268,8 @@ static int setup_func_entry(struct bpf_verifier_env *env, int subprog, int calls
}
if (state->frame[state->curframe + 1]) {
- verbose(env, "verifier bug. Frame %d already allocated\n",
- state->curframe + 1);
+ verifier_bug(env, "Frame %d already allocated",
+ state->curframe + 1);
return -EFAULT;
}
@@ -10401,8 +10383,8 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
if (err)
return err;
} else {
- bpf_log(log, "verifier bug: unrecognized arg#%d type %d\n",
- i, arg->arg_type);
+ verifier_bug(env, "unrecognized arg#%d type %d",
+ i, arg->arg_type);
return -EFAULT;
}
}
@@ -10465,13 +10447,13 @@ static int push_callback_call(struct bpf_verifier_env *env, struct bpf_insn *ins
env->subprog_info[subprog].is_cb = true;
if (bpf_pseudo_kfunc_call(insn) &&
!is_callback_calling_kfunc(insn->imm)) {
- verbose(env, "verifier bug: kfunc %s#%d not marked as callback-calling\n",
- func_id_name(insn->imm), insn->imm);
+ verifier_bug(env, "kfunc %s#%d not marked as callback-calling",
+ func_id_name(insn->imm), insn->imm);
return -EFAULT;
} else if (!bpf_pseudo_kfunc_call(insn) &&
!is_callback_calling_function(insn->imm)) { /* helper */
- verbose(env, "verifier bug: helper %s#%d not marked as callback-calling\n",
- func_id_name(insn->imm), insn->imm);
+ verifier_bug(env, "helper %s#%d not marked as callback-calling",
+ func_id_name(insn->imm), insn->imm);
return -EFAULT;
}
@@ -10523,10 +10505,8 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
target_insn = *insn_idx + insn->imm + 1;
subprog = find_subprog(env, target_insn);
- if (subprog < 0) {
- verbose(env, "verifier bug. No program starts at insn %d\n", target_insn);
+ if (verifier_bug_if(subprog < 0, env, "No program starts at insn %d", target_insn))
return -EFAULT;
- }
caller = state->frame[state->curframe];
err = btf_check_subprog_call(env, subprog, caller->regs);
@@ -11125,7 +11105,7 @@ static int check_bpf_snprintf_call(struct bpf_verifier_env *env,
err = fmt_map->ops->map_direct_value_addr(fmt_map, &fmt_addr,
fmt_map_off);
if (err) {
- verbose(env, "verifier bug\n");
+ verbose(env, "failed to retrieve map value address\n");
return -EFAULT;
}
fmt = (char *)(long)fmt_addr + fmt_map_off;
@@ -19706,10 +19686,9 @@ static int do_check(struct bpf_verifier_env *env)
return err;
break;
} else {
- if (WARN_ON_ONCE(env->cur_state->loop_entry)) {
- verbose(env, "verifier bug: env->cur_state->loop_entry != NULL\n");
+ if (verifier_bug_if(env->cur_state->loop_entry, env,
+ "broken loop detection"))
return -EFAULT;
- }
do_print_state = true;
continue;
}
@@ -20767,10 +20746,9 @@ static int opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env,
if (bpf_pseudo_kfunc_call(&insn))
continue;
- if (WARN_ON(load_reg == -1)) {
- verbose(env, "verifier bug. zext_dst is set, but no reg is defined\n");
+ if (verifier_bug_if(load_reg == -1, env,
+ "zext_dst is set, but no reg is defined"))
return -EFAULT;
- }
zext_patch[0] = insn;
zext_patch[1].dst_reg = load_reg;
@@ -21087,11 +21065,9 @@ static int jit_subprogs(struct bpf_verifier_env *env)
* propagated in any case.
*/
subprog = find_subprog(env, i + insn->imm + 1);
- if (subprog < 0) {
- WARN_ONCE(1, "verifier bug. No program starts at insn %d\n",
- i + insn->imm + 1);
+ if (verifier_bug_if(subprog < 0, env, "No program starts at insn %d",
+ i + insn->imm + 1))
return -EFAULT;
- }
/* temporarily remember subprog id inside insn instead of
* aux_data, since next loop will split up all insns into funcs
*/
@@ -22454,7 +22430,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
continue;
/* We need two slots in case timed may_goto is supported. */
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;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next v4] bpf: WARN_ONCE on verifier bugs
2025-05-16 9:34 [PATCH bpf-next v4] bpf: WARN_ONCE on verifier bugs Paul Chaignon
@ 2025-05-16 16:14 ` Andrii Nakryiko
2025-05-19 12:21 ` Paul Chaignon
0 siblings, 1 reply; 4+ messages in thread
From: Andrii Nakryiko @ 2025-05-16 16:14 UTC (permalink / raw)
To: Paul Chaignon; +Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
On Fri, May 16, 2025 at 2:34 AM Paul Chaignon <paul.chaignon@gmail.com> wrote:
>
> 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.
>
> Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> ---
> Changes in v4:
> - Evaluate condition once and stringify it, as suggested by Alexei.
> - Use verifier_bug_if instead of verifier_bug where it can help
> disambiguate the callsite or shorten the message.
> - Add newline character in verifier_bug_if directly.
> Changes in v3:
> - Introduce and use verifier_bug_if, as suggested by Andrii.
> Changes in v2:
> - Introduce a new BPF_WARN_ONCE macro, with WARN_ONCE conditioned on
> CONFIG_DEBUG_KERNEL, as per reviews.
> - Use the new helper function for verifier bugs missed in v1,
> particularly around backtracking.
>
> include/linux/bpf.h | 6 ++
> include/linux/bpf_verifier.h | 11 +++
> kernel/bpf/btf.c | 4 +-
> kernel/bpf/verifier.c | 140 +++++++++++++++--------------------
> 4 files changed, 77 insertions(+), 84 deletions(-)
>
LGTM overall, left a few comments below, please take a look
Acked-by: Andrii Nakryiko <andrii@kernel.org>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 83c56f40842b..5b25d278409b 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -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)
> {
> switch (type) {
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index cedd66867ecf..7edb15830132 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -839,6 +839,17 @@ __printf(3, 4) void verbose_linfo(struct bpf_verifier_env *env,
> u32 insn_off,
> const char *prefix_fmt, ...);
>
> +#define verifier_bug_if(cond, env, fmt, args...) \
> + ({ \
> + bool __cond = unlikely(cond); \
> + if (__cond) { \
this might be equivalent in terms of code generation, but I'd expect
unlikely() to be inside the if()
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)
> {
> struct bpf_verifier_state *cur = env->cur_state;
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 6b21ca67070c..0f7828380895 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -7659,7 +7659,7 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
> return 0;
>
> if (!prog->aux->func_info) {
> - bpf_log(log, "Verifier bug\n");
> + verifier_bug(env, "func_info undefined");
> 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);
>
> 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;
> }
> if (prog_type == BPF_PROG_TYPE_EXT)
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index f6d3655b3a7a..cec35daf2b77 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -1924,11 +1924,9 @@ static struct bpf_verifier_state *get_loop_entry(struct bpf_verifier_env *env,
> u32 steps = 0;
>
> while (topmost && topmost->loop_entry) {
> - if (steps++ > st->dfs_depth) {
> - WARN_ONCE(true, "verifier bug: infinite loop in get_loop_entry\n");
> - verbose(env, "verifier bug: infinite loop in get_loop_entry()\n");
> + if (verifier_bug_if(steps++ > st->dfs_depth, env,
> + "infinite loop"))
nit: I'd keep it single-line (it probably fits 100 characters limit)
> return ERR_PTR(-EFAULT);
> - }
> topmost = topmost->loop_entry;
> }
> return topmost;
[...]
> @@ -3999,8 +3995,7 @@ static inline int bt_subprog_enter(struct backtrack_state *bt)
> static inline int bt_subprog_exit(struct backtrack_state *bt)
> {
> if (bt->frame == 0) {
> - verbose(bt->env, "BUG subprog exit from frame 0\n");
> - WARN_ONCE(1, "verifier backtracking bug");
> + verifier_bug(bt->env, "subprog exit from frame 0");
> return -EFAULT;
> }
> bt->frame--;
> @@ -4278,14 +4273,15 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
> * should be literally next instruction in
> * 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,
> * so for global func call it shouldn't be set
> * anymore
> */
> if (bt_reg_mask(bt) & BPF_REGMASK_ARGS) {
> - verbose(env, "BUG regs %x\n", bt_reg_mask(bt));
> - WARN_ONCE(1, "verifier backtracking bug");
> + verifier_bug(env, "scratch reg set: regs %x",
"global subprog unexpected regs %x"
> + bt_reg_mask(bt));
> return -EFAULT;
> }
> /* global subprog always sets R0 */
> @@ -4299,16 +4295,16 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
> * the current frame should be zero by now
> */
> if (bt_reg_mask(bt) & ~BPF_REGMASK_ARGS) {
> - verbose(env, "BUG regs %x\n", bt_reg_mask(bt));
> - WARN_ONCE(1, "verifier backtracking bug");
> + verifier_bug(env, "unexpected precise regs %x",
"static subprog unexpected regs %x"
(note, user is not expected to really make sense out of this, it's
just for reporting to us and our debugging, so let's make messages
distinct, but they don't necessarily need to be precise, IMO)
> + bt_reg_mask(bt));
> return -EFAULT;
> }
> /* we are now tracking register spills correctly,
> * so any instance of leftover slots is a bug
> */
> if (bt_stack_mask(bt) != 0) {
> - verbose(env, "BUG stack slots %llx\n", bt_stack_mask(bt));
> - WARN_ONCE(1, "verifier backtracking bug (subprog leftover stack slots)");
> + verifier_bug(env, "subprog leftover stack slots %llx",
> + bt_stack_mask(bt));
good enough as is, but we can add "static " for consistency with the above
> return -EFAULT;
> }
> /* 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
> */
> if (bt_reg_mask(bt) & ~BPF_REGMASK_ARGS) {
> - verbose(env, "BUG regs %x\n", bt_reg_mask(bt));
> - WARN_ONCE(1, "verifier backtracking bug");
> + verifier_bug(env, "unexpected precise regs %x",
> + bt_reg_mask(bt));
"callback unexpected regs %x"
> return -EFAULT;
> }
> if (bt_stack_mask(bt) != 0) {
> - verbose(env, "BUG stack slots %llx\n", bt_stack_mask(bt));
> - WARN_ONCE(1, "verifier backtracking bug (callback leftover stack slots)");
> + verifier_bug(env, "callback leftover stack slots %llx",
> + bt_stack_mask(bt));
> return -EFAULT;
> }
> /* clear r1-r5 in callback subprog's mask */
> @@ -4359,8 +4355,7 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
> /* if backtracing was looking for registers R1-R5
oops, "backtracking" :)
> * they should have been found already.
> */
> - verbose(env, "BUG regs %x\n", bt_reg_mask(bt));
> - WARN_ONCE(1, "verifier backtracking bug");
> + verifier_bug(env, "regs not found %x", bt_reg_mask(bt));
"call unexpected regs %x"
> return -EFAULT;
> }
> } else if (opcode == BPF_EXIT) {
> @@ -4378,8 +4373,7 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
> for (i = BPF_REG_1; i <= BPF_REG_5; i++)
> bt_clear_reg(bt, i);
> if (bt_reg_mask(bt) & BPF_REGMASK_ARGS) {
> - verbose(env, "BUG regs %x\n", bt_reg_mask(bt));
> - WARN_ONCE(1, "verifier backtracking bug");
> + verifier_bug(env, "regs not found %x", bt_reg_mask(bt));
for this one and a few above, I'd probably leave the "backtracking"
word in the message, so something like
"backtracking exit unexpected regs %x"
> return -EFAULT;
> }
>
> @@ -4720,9 +4714,8 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int regno)
> return 0;
> }
>
> - verbose(env, "BUG backtracking func entry subprog %d reg_mask %x stack_mask %llx\n",
> - st->frame[0]->subprogno, bt_reg_mask(bt), bt_stack_mask(bt));
> - WARN_ONCE(1, "verifier backtracking bug");
> + 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));
> return -EFAULT;
> }
>
> @@ -4751,17 +4744,14 @@ static int __mark_chain_precision(struct bpf_verifier_env *env, int regno)
> i = get_prev_insn_idx(env, st, i, hist_start, &hist_end);
> if (i == -ENOENT)
> break;
> - if (i >= env->prog->len) {
> + if (verifier_bug_if(i >= env->prog->len, env, "backtracking idx %d", i))
this is the case where I'd leaver verifier_bug_if() inside the if() {}
itself, at least due to that long comment that actually describes the
bug context, but as I said, it's minor
> /* This can happen if backtracking reached insn 0
> * and there are still reg_mask or stack_mask
> * to backtrack.
> * It means the backtracking missed the spot where
> * particular register was initialized with a constant.
> */
> - verbose(env, "BUG backtracking idx %d\n", i);
> - WARN_ONCE(1, "verifier backtracking bug");
> return -EFAULT;
> - }
> }
> st = st->parent;
> if (!st)
[...]
> @@ -10286,8 +10268,8 @@ static int setup_func_entry(struct bpf_verifier_env *env, int subprog, int calls
> }
>
> if (state->frame[state->curframe + 1]) {
> - verbose(env, "verifier bug. Frame %d already allocated\n",
> - state->curframe + 1);
> + verifier_bug(env, "Frame %d already allocated",
> + state->curframe + 1);
nit: single line
> return -EFAULT;
> }
>
> @@ -10401,8 +10383,8 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
> if (err)
> return err;
> } else {
> - bpf_log(log, "verifier bug: unrecognized arg#%d type %d\n",
> - i, arg->arg_type);
> + verifier_bug(env, "unrecognized arg#%d type %d",
> + i, arg->arg_type);
single line, if possible
> return -EFAULT;
> }
> }
[...]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next v4] bpf: WARN_ONCE on verifier bugs
@ 2025-05-17 4:12 kernel test robot
0 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2025-05-17 4:12 UTC (permalink / raw)
To: oe-kbuild; +Cc: lkp, Dan Carpenter
BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <aCcGpxnlfOOiOJ-b@mail.gmail.com>
References: <aCcGpxnlfOOiOJ-b@mail.gmail.com>
TO: Paul Chaignon <paul.chaignon@gmail.com>
TO: bpf@vger.kernel.org
CC: Alexei Starovoitov <ast@kernel.org>
CC: Daniel Borkmann <daniel@iogearbox.net>
CC: Andrii Nakryiko <andrii@kernel.org>
Hi Paul,
kernel test robot noticed the following build warnings:
[auto build test WARNING on bpf-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Paul-Chaignon/bpf-WARN_ONCE-on-verifier-bugs/20250516-174044
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/aCcGpxnlfOOiOJ-b%40mail.gmail.com
patch subject: [PATCH bpf-next v4] bpf: WARN_ONCE on verifier bugs
:::::: branch date: 18 hours ago
:::::: commit date: 18 hours ago
config: i386-randconfig-141-20250517 (https://download.01.org/0day-ci/archive/20250517/202505171244.4j4x4rqt-lkp@intel.com/config)
compiler: clang version 20.1.2 (https://github.com/llvm/llvm-project 58df0ef89dd64126512e4ee27b4ac3fd8ddf6247)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Closes: https://lore.kernel.org/r/202505171244.4j4x4rqt-lkp@intel.com/
smatch warnings:
kernel/bpf/btf.c:7662 btf_prepare_func_args() warn: check likely/unlikely parentheses
kernel/bpf/verifier.c:4283 backtrack_insn() warn: check likely/unlikely parentheses
kernel/bpf/verifier.c:3998 bt_subprog_exit() warn: check likely/unlikely parentheses
kernel/bpf/verifier.c:3988 bt_subprog_enter() warn: check likely/unlikely parentheses
kernel/bpf/verifier.c:4717 __mark_chain_precision() warn: check likely/unlikely parentheses
kernel/bpf/verifier.c:6557 check_max_stack_depth_subprog() warn: check likely/unlikely parentheses
kernel/bpf/verifier.c:8399 process_timer_func() warn: check likely/unlikely parentheses
kernel/bpf/verifier.c:10271 setup_func_entry() warn: check likely/unlikely parentheses
kernel/bpf/verifier.c:10386 btf_check_func_arg_match() warn: check likely/unlikely parentheses
kernel/bpf/verifier.c:10450 push_callback_call() warn: check likely/unlikely parentheses
kernel/bpf/verifier.c:22433 do_misc_fixups() warn: check likely/unlikely parentheses
vim +7662 kernel/bpf/btf.c
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7636
4ba1d0f2341413 Andrii Nakryiko 2023-12-14 7637 /* Process BTF of a function to produce high-level expectation of function
4ba1d0f2341413 Andrii Nakryiko 2023-12-14 7638 * arguments (like ARG_PTR_TO_CTX, or ARG_PTR_TO_MEM, etc). This information
4ba1d0f2341413 Andrii Nakryiko 2023-12-14 7639 * is cached in subprog info for reuse.
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7640 * Returns:
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7641 * EFAULT - there is a verifier bug. Abort verification.
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7642 * EINVAL - cannot convert BTF.
4ba1d0f2341413 Andrii Nakryiko 2023-12-14 7643 * 0 - Successfully processed BTF and constructed argument expectations.
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7644 */
4ba1d0f2341413 Andrii Nakryiko 2023-12-14 7645 int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7646 {
e26080d0da87f2 Andrii Nakryiko 2023-12-14 7647 bool is_global = subprog_aux(env, subprog)->linkage == BTF_FUNC_GLOBAL;
4ba1d0f2341413 Andrii Nakryiko 2023-12-14 7648 struct bpf_subprog_info *sub = subprog_info(env, subprog);
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7649 struct bpf_verifier_log *log = &env->log;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7650 struct bpf_prog *prog = env->prog;
be8704ff07d237 Alexei Starovoitov 2020-01-20 7651 enum bpf_prog_type prog_type = prog->type;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7652 struct btf *btf = prog->aux->btf;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7653 const struct btf_param *args;
94e1c70a34523b Andrii Nakryiko 2023-12-14 7654 const struct btf_type *t, *ref_t, *fn_t;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7655 u32 i, nargs, btf_id;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7656 const char *tname;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7657
4ba1d0f2341413 Andrii Nakryiko 2023-12-14 7658 if (sub->args_cached)
4ba1d0f2341413 Andrii Nakryiko 2023-12-14 7659 return 0;
4ba1d0f2341413 Andrii Nakryiko 2023-12-14 7660
e26080d0da87f2 Andrii Nakryiko 2023-12-14 7661 if (!prog->aux->func_info) {
c59cec93356194 Paul Chaignon 2025-05-16 @7662 verifier_bug(env, "func_info undefined");
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7663 return -EFAULT;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7664 }
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7665
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7666 btf_id = prog->aux->func_info[subprog].type_id;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7667 if (!btf_id) {
e26080d0da87f2 Andrii Nakryiko 2023-12-14 7668 if (!is_global) /* not fatal for static funcs */
e26080d0da87f2 Andrii Nakryiko 2023-12-14 7669 return -EINVAL;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7670 bpf_log(log, "Global functions need valid BTF\n");
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7671 return -EFAULT;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7672 }
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7673
94e1c70a34523b Andrii Nakryiko 2023-12-14 7674 fn_t = btf_type_by_id(btf, btf_id);
94e1c70a34523b Andrii Nakryiko 2023-12-14 7675 if (!fn_t || !btf_type_is_func(fn_t)) {
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7676 /* These checks were already done by the verifier while loading
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7677 * struct bpf_func_info
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7678 */
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7679 bpf_log(log, "BTF of func#%d doesn't point to KIND_FUNC\n",
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7680 subprog);
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7681 return -EFAULT;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7682 }
94e1c70a34523b Andrii Nakryiko 2023-12-14 7683 tname = btf_name_by_offset(btf, fn_t->name_off);
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7684
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7685 if (prog->aux->func_info_aux[subprog].unreliable) {
c59cec93356194 Paul Chaignon 2025-05-16 7686 verifier_bug(env, "unreliable BTF for function %s()", tname);
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7687 return -EFAULT;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7688 }
be8704ff07d237 Alexei Starovoitov 2020-01-20 7689 if (prog_type == BPF_PROG_TYPE_EXT)
3aac1ead5eb6b7 Toke Høiland-Jørgensen 2020-09-29 7690 prog_type = prog->aux->dst_prog->type;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7691
94e1c70a34523b Andrii Nakryiko 2023-12-14 7692 t = btf_type_by_id(btf, fn_t->type);
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7693 if (!t || !btf_type_is_func_proto(t)) {
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7694 bpf_log(log, "Invalid type of function %s()\n", tname);
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7695 return -EFAULT;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7696 }
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7697 args = (const struct btf_param *)(t + 1);
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7698 nargs = btf_type_vlen(t);
523a4cf491b3c9 Dmitrii Banshchikov 2021-02-26 7699 if (nargs > MAX_BPF_FUNC_REG_ARGS) {
1eb986746a6795 Andrii Nakryiko 2024-02-02 7700 if (!is_global)
1eb986746a6795 Andrii Nakryiko 2024-02-02 7701 return -EINVAL;
523a4cf491b3c9 Dmitrii Banshchikov 2021-02-26 7702 bpf_log(log, "Global function %s() with %d > %d args. Buggy compiler.\n",
523a4cf491b3c9 Dmitrii Banshchikov 2021-02-26 7703 tname, nargs, MAX_BPF_FUNC_REG_ARGS);
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7704 return -EINVAL;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7705 }
b9ae0c9dd0aca7 Kumar Kartikeya Dwivedi 2023-09-13 7706 /* check that function returns int, exception cb also requires this */
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7707 t = btf_type_by_id(btf, t->type);
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7708 while (btf_type_is_modifier(t))
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7709 t = btf_type_by_id(btf, t->type);
6089fb325cf737 Yonghong Song 2022-06-06 7710 if (!btf_type_is_int(t) && !btf_is_any_enum(t)) {
1eb986746a6795 Andrii Nakryiko 2024-02-02 7711 if (!is_global)
1eb986746a6795 Andrii Nakryiko 2024-02-02 7712 return -EINVAL;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7713 bpf_log(log,
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7714 "Global function %s() doesn't return scalar. Only those are supported.\n",
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7715 tname);
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7716 return -EINVAL;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7717 }
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7718 /* Convert BTF function arguments into verifier types.
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7719 * Only PTR_TO_CTX and SCALAR are supported atm.
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7720 */
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7721 for (i = 0; i < nargs; i++) {
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7722 u32 tags = 0;
522bb2c1f82b12 Andrii Nakryiko 2024-01-04 7723 int id = 0;
94e1c70a34523b Andrii Nakryiko 2023-12-14 7724
94e1c70a34523b Andrii Nakryiko 2023-12-14 7725 /* 'arg:<tag>' decl_tag takes precedence over derivation of
94e1c70a34523b Andrii Nakryiko 2023-12-14 7726 * register type from BTF type itself
94e1c70a34523b Andrii Nakryiko 2023-12-14 7727 */
522bb2c1f82b12 Andrii Nakryiko 2024-01-04 7728 while ((id = btf_find_next_decl_tag(btf, fn_t, i, "arg:", id)) > 0) {
522bb2c1f82b12 Andrii Nakryiko 2024-01-04 7729 const struct btf_type *tag_t = btf_type_by_id(btf, id);
522bb2c1f82b12 Andrii Nakryiko 2024-01-04 7730 const char *tag = __btf_name_by_offset(btf, tag_t->name_off) + 4;
522bb2c1f82b12 Andrii Nakryiko 2024-01-04 7731
94e1c70a34523b Andrii Nakryiko 2023-12-14 7732 /* disallow arg tags in static subprogs */
94e1c70a34523b Andrii Nakryiko 2023-12-14 7733 if (!is_global) {
94e1c70a34523b Andrii Nakryiko 2023-12-14 7734 bpf_log(log, "arg#%d type tag is not supported in static functions\n", i);
94e1c70a34523b Andrii Nakryiko 2023-12-14 7735 return -EOPNOTSUPP;
94e1c70a34523b Andrii Nakryiko 2023-12-14 7736 }
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7737
94e1c70a34523b Andrii Nakryiko 2023-12-14 7738 if (strcmp(tag, "ctx") == 0) {
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7739 tags |= ARG_TAG_CTX;
e2b3c4ff5d183d Andrii Nakryiko 2024-01-29 7740 } else if (strcmp(tag, "trusted") == 0) {
e2b3c4ff5d183d Andrii Nakryiko 2024-01-29 7741 tags |= ARG_TAG_TRUSTED;
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7742 } else if (strcmp(tag, "nonnull") == 0) {
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7743 tags |= ARG_TAG_NONNULL;
8f2b44cd9d69ec Andrii Nakryiko 2024-01-29 7744 } else if (strcmp(tag, "nullable") == 0) {
8f2b44cd9d69ec Andrii Nakryiko 2024-01-29 7745 tags |= ARG_TAG_NULLABLE;
2edc3de6fb6509 Alexei Starovoitov 2024-03-07 7746 } else if (strcmp(tag, "arena") == 0) {
2edc3de6fb6509 Alexei Starovoitov 2024-03-07 7747 tags |= ARG_TAG_ARENA;
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7748 } else {
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7749 bpf_log(log, "arg#%d has unsupported set of tags\n", i);
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7750 return -EOPNOTSUPP;
94e1c70a34523b Andrii Nakryiko 2023-12-14 7751 }
94e1c70a34523b Andrii Nakryiko 2023-12-14 7752 }
522bb2c1f82b12 Andrii Nakryiko 2024-01-04 7753 if (id != -ENOENT) {
522bb2c1f82b12 Andrii Nakryiko 2024-01-04 7754 bpf_log(log, "arg#%d type tag fetching failure: %d\n", i, id);
522bb2c1f82b12 Andrii Nakryiko 2024-01-04 7755 return id;
522bb2c1f82b12 Andrii Nakryiko 2024-01-04 7756 }
94e1c70a34523b Andrii Nakryiko 2023-12-14 7757
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7758 t = btf_type_by_id(btf, args[i].type);
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7759 while (btf_type_is_modifier(t))
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7760 t = btf_type_by_id(btf, t->type);
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7761 if (!btf_type_is_ptr(t))
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7762 goto skip_pointer;
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7763
fb5b86cfd4ef21 Andrii Nakryiko 2024-02-12 7764 if ((tags & ARG_TAG_CTX) || btf_is_prog_ctx_type(log, btf, t, prog_type, i)) {
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7765 if (tags & ~ARG_TAG_CTX) {
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7766 bpf_log(log, "arg#%d has invalid combination of tags\n", i);
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7767 return -EINVAL;
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7768 }
add9c58cd44e88 Andrii Nakryiko 2024-01-25 7769 if ((tags & ARG_TAG_CTX) &&
add9c58cd44e88 Andrii Nakryiko 2024-01-25 7770 btf_validate_prog_ctx_type(log, btf, t, i, prog_type,
add9c58cd44e88 Andrii Nakryiko 2024-01-25 7771 prog->expected_attach_type))
add9c58cd44e88 Andrii Nakryiko 2024-01-25 7772 return -EINVAL;
4ba1d0f2341413 Andrii Nakryiko 2023-12-14 7773 sub->args[i].arg_type = ARG_PTR_TO_CTX;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7774 continue;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7775 }
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7776 if (btf_is_dynptr_ptr(btf, t)) {
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7777 if (tags) {
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7778 bpf_log(log, "arg#%d has invalid combination of tags\n", i);
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7779 return -EINVAL;
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7780 }
a64bfe618665ea Andrii Nakryiko 2023-12-14 7781 sub->args[i].arg_type = ARG_PTR_TO_DYNPTR | MEM_RDONLY;
a64bfe618665ea Andrii Nakryiko 2023-12-14 7782 continue;
a64bfe618665ea Andrii Nakryiko 2023-12-14 7783 }
e2b3c4ff5d183d Andrii Nakryiko 2024-01-29 7784 if (tags & ARG_TAG_TRUSTED) {
e2b3c4ff5d183d Andrii Nakryiko 2024-01-29 7785 int kern_type_id;
e2b3c4ff5d183d Andrii Nakryiko 2024-01-29 7786
e2b3c4ff5d183d Andrii Nakryiko 2024-01-29 7787 if (tags & ARG_TAG_NONNULL) {
e2b3c4ff5d183d Andrii Nakryiko 2024-01-29 7788 bpf_log(log, "arg#%d has invalid combination of tags\n", i);
e2b3c4ff5d183d Andrii Nakryiko 2024-01-29 7789 return -EINVAL;
e2b3c4ff5d183d Andrii Nakryiko 2024-01-29 7790 }
e2b3c4ff5d183d Andrii Nakryiko 2024-01-29 7791
e2b3c4ff5d183d Andrii Nakryiko 2024-01-29 7792 kern_type_id = btf_get_ptr_to_btf_id(log, i, btf, t);
e2b3c4ff5d183d Andrii Nakryiko 2024-01-29 7793 if (kern_type_id < 0)
e2b3c4ff5d183d Andrii Nakryiko 2024-01-29 7794 return kern_type_id;
e2b3c4ff5d183d Andrii Nakryiko 2024-01-29 7795
e2b3c4ff5d183d Andrii Nakryiko 2024-01-29 7796 sub->args[i].arg_type = ARG_PTR_TO_BTF_ID | PTR_TRUSTED;
8f2b44cd9d69ec Andrii Nakryiko 2024-01-29 7797 if (tags & ARG_TAG_NULLABLE)
8f2b44cd9d69ec Andrii Nakryiko 2024-01-29 7798 sub->args[i].arg_type |= PTR_MAYBE_NULL;
e2b3c4ff5d183d Andrii Nakryiko 2024-01-29 7799 sub->args[i].btf_id = kern_type_id;
e2b3c4ff5d183d Andrii Nakryiko 2024-01-29 7800 continue;
e2b3c4ff5d183d Andrii Nakryiko 2024-01-29 7801 }
2edc3de6fb6509 Alexei Starovoitov 2024-03-07 7802 if (tags & ARG_TAG_ARENA) {
2edc3de6fb6509 Alexei Starovoitov 2024-03-07 7803 if (tags & ~ARG_TAG_ARENA) {
2edc3de6fb6509 Alexei Starovoitov 2024-03-07 7804 bpf_log(log, "arg#%d arena cannot be combined with any other tags\n", i);
2edc3de6fb6509 Alexei Starovoitov 2024-03-07 7805 return -EINVAL;
2edc3de6fb6509 Alexei Starovoitov 2024-03-07 7806 }
2edc3de6fb6509 Alexei Starovoitov 2024-03-07 7807 sub->args[i].arg_type = ARG_PTR_TO_ARENA;
2edc3de6fb6509 Alexei Starovoitov 2024-03-07 7808 continue;
2edc3de6fb6509 Alexei Starovoitov 2024-03-07 7809 }
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7810 if (is_global) { /* generic user data pointer */
e26080d0da87f2 Andrii Nakryiko 2023-12-14 7811 u32 mem_size;
e5069b9c23b385 Dmitrii Banshchikov 2021-02-13 7812
8f2b44cd9d69ec Andrii Nakryiko 2024-01-29 7813 if (tags & ARG_TAG_NULLABLE) {
8f2b44cd9d69ec Andrii Nakryiko 2024-01-29 7814 bpf_log(log, "arg#%d has invalid combination of tags\n", i);
8f2b44cd9d69ec Andrii Nakryiko 2024-01-29 7815 return -EINVAL;
8f2b44cd9d69ec Andrii Nakryiko 2024-01-29 7816 }
8f2b44cd9d69ec Andrii Nakryiko 2024-01-29 7817
e5069b9c23b385 Dmitrii Banshchikov 2021-02-13 7818 t = btf_type_skip_modifiers(btf, t->type, NULL);
4ba1d0f2341413 Andrii Nakryiko 2023-12-14 7819 ref_t = btf_resolve_size(btf, t, &mem_size);
e5069b9c23b385 Dmitrii Banshchikov 2021-02-13 7820 if (IS_ERR(ref_t)) {
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7821 bpf_log(log, "arg#%d reference type('%s %s') size cannot be determined: %ld\n",
e5069b9c23b385 Dmitrii Banshchikov 2021-02-13 7822 i, btf_type_str(t), btf_name_by_offset(btf, t->name_off),
e5069b9c23b385 Dmitrii Banshchikov 2021-02-13 7823 PTR_ERR(ref_t));
e5069b9c23b385 Dmitrii Banshchikov 2021-02-13 7824 return -EINVAL;
e5069b9c23b385 Dmitrii Banshchikov 2021-02-13 7825 }
e5069b9c23b385 Dmitrii Banshchikov 2021-02-13 7826
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7827 sub->args[i].arg_type = ARG_PTR_TO_MEM | PTR_MAYBE_NULL;
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7828 if (tags & ARG_TAG_NONNULL)
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7829 sub->args[i].arg_type &= ~PTR_MAYBE_NULL;
4ba1d0f2341413 Andrii Nakryiko 2023-12-14 7830 sub->args[i].mem_size = mem_size;
e5069b9c23b385 Dmitrii Banshchikov 2021-02-13 7831 continue;
e5069b9c23b385 Dmitrii Banshchikov 2021-02-13 7832 }
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7833
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7834 skip_pointer:
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7835 if (tags) {
54c11ec4935a61 Andrii Nakryiko 2024-01-04 7836 bpf_log(log, "arg#%d has pointer tag, but is not a pointer type\n", i);
94e1c70a34523b Andrii Nakryiko 2023-12-14 7837 return -EINVAL;
94e1c70a34523b Andrii Nakryiko 2023-12-14 7838 }
18810ad3929ff6 Andrii Nakryiko 2024-01-04 7839 if (btf_type_is_int(t) || btf_is_any_enum(t)) {
18810ad3929ff6 Andrii Nakryiko 2024-01-04 7840 sub->args[i].arg_type = ARG_ANYTHING;
18810ad3929ff6 Andrii Nakryiko 2024-01-04 7841 continue;
18810ad3929ff6 Andrii Nakryiko 2024-01-04 7842 }
1eb986746a6795 Andrii Nakryiko 2024-02-02 7843 if (!is_global)
1eb986746a6795 Andrii Nakryiko 2024-02-02 7844 return -EINVAL;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7845 bpf_log(log, "Arg#%d type %s in %s() is not supported yet.\n",
571f9738bfb3d4 Peilin Ye 2022-09-16 7846 i, btf_type_str(t), tname);
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7847 return -EINVAL;
51c39bb1d5d105 Alexei Starovoitov 2020-01-09 7848 }
4ba1d0f2341413 Andrii Nakryiko 2023-12-14 7849
4ba1d0f2341413 Andrii Nakryiko 2023-12-14 7850 sub->arg_cnt = nargs;
4ba1d0f2341413 Andrii Nakryiko 2023-12-14 7851 sub->args_cached = true;
4ba1d0f2341413 Andrii Nakryiko 2023-12-14 7852
8c1b6e69dcc1e1 Alexei Starovoitov 2019-11-14 7853 return 0;
8c1b6e69dcc1e1 Alexei Starovoitov 2019-11-14 7854 }
8c1b6e69dcc1e1 Alexei Starovoitov 2019-11-14 7855
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next v4] bpf: WARN_ONCE on verifier bugs
2025-05-16 16:14 ` Andrii Nakryiko
@ 2025-05-19 12:21 ` Paul Chaignon
0 siblings, 0 replies; 4+ messages in thread
From: Paul Chaignon @ 2025-05-19 12:21 UTC (permalink / raw)
To: Andrii Nakryiko; +Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
On Fri, May 16, 2025 at 09:14:40AM -0700, Andrii Nakryiko wrote:
> On Fri, May 16, 2025 at 2:34 AM Paul Chaignon <paul.chaignon@gmail.com> wrote:
> >
> > 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.
> >
> > Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> > ---
> > Changes in v4:
> > - Evaluate condition once and stringify it, as suggested by Alexei.
> > - Use verifier_bug_if instead of verifier_bug where it can help
> > disambiguate the callsite or shorten the message.
> > - Add newline character in verifier_bug_if directly.
> > Changes in v3:
> > - Introduce and use verifier_bug_if, as suggested by Andrii.
> > Changes in v2:
> > - Introduce a new BPF_WARN_ONCE macro, with WARN_ONCE conditioned on
> > CONFIG_DEBUG_KERNEL, as per reviews.
> > - Use the new helper function for verifier bugs missed in v1,
> > particularly around backtracking.
> >
> > include/linux/bpf.h | 6 ++
> > include/linux/bpf_verifier.h | 11 +++
> > kernel/bpf/btf.c | 4 +-
> > kernel/bpf/verifier.c | 140 +++++++++++++++--------------------
> > 4 files changed, 77 insertions(+), 84 deletions(-)
> >
>
> LGTM overall, left a few comments below, please take a look
>
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
Thanks a lot for the detailed review!
>
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 83c56f40842b..5b25d278409b 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -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)
> > {
> > switch (type) {
> > diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> > index cedd66867ecf..7edb15830132 100644
> > --- a/include/linux/bpf_verifier.h
> > +++ b/include/linux/bpf_verifier.h
> > @@ -839,6 +839,17 @@ __printf(3, 4) void verbose_linfo(struct bpf_verifier_env *env,
> > u32 insn_off,
> > const char *prefix_fmt, ...);
> >
> > +#define verifier_bug_if(cond, env, fmt, args...) \
> > + ({ \
> > + bool __cond = unlikely(cond); \
> > + if (__cond) { \
>
> this might be equivalent in terms of code generation, but I'd expect
> unlikely() to be inside the if()
>
> bool __cond = (cond);
> if (unlikely(__cond)) { ... }
I was worried the compiler may not take the unlikely into account when
doing if (verifier_bug_if(...)). I checked with a small example
involving a similar macro and the generated code is indeed the exact
same. I'll stick to the usual style, as suggested.
[...]
> > + bt_reg_mask(bt));
> > return -EFAULT;
> > }
> > /* global subprog always sets R0 */
> > @@ -4299,16 +4295,16 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
> > * the current frame should be zero by now
> > */
> > if (bt_reg_mask(bt) & ~BPF_REGMASK_ARGS) {
> > - verbose(env, "BUG regs %x\n", bt_reg_mask(bt));
> > - WARN_ONCE(1, "verifier backtracking bug");
> > + verifier_bug(env, "unexpected precise regs %x",
>
> "static subprog unexpected regs %x"
>
> (note, user is not expected to really make sense out of this, it's
> just for reporting to us and our debugging, so let's make messages
> distinct, but they don't necessarily need to be precise, IMO)
That makes sense. Considering this, I went back over the four identical
"No program starts at insn" error messages and tweaked them a bit based
on context. All error messages should now be unique.
[...]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-05-19 12:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-16 9:34 [PATCH bpf-next v4] bpf: WARN_ONCE on verifier bugs Paul Chaignon
2025-05-16 16:14 ` Andrii Nakryiko
2025-05-19 12:21 ` Paul Chaignon
-- strict thread matches above, loose matches on Subject: below --
2025-05-17 4:12 kernel test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.