Re: [PATCH bpf-next v3 1/4] bpf: Add support for verifier warning messages

On Sat, 18 Apr 2026 at 18:42, <[email protected]> wrote:
>
> Sashiko AI review found 2 potential issue(s):
> - [High] Unused static function `warn()` breaks the build and bisection.
> - [High] Disabling log reset completely after a warning leads to log buffer explosion and overwrites the warning when combined with `BPF_LOG_LEVEL1`.
> --
>
> commit 35e5ff3f696c81863084f9e6fb58225f8242f3e0
> Author: Kumar Kartikeya Dwivedi <[email protected]>
>
> bpf: Add support for verifier warning messages
>
> This commit adds a mode where log_level 16 can be used to receive warnings
> and have an associated log buffer. It introduces a warn function and
> aggregates warnings to decide whether to reset the log.
>
> > Add a warn() macro that emits messages to log
> > buffer without any restrictions, aggregate the warnings emitted, and
>
> This isn't a bug, but the commit message states it adds a warn() macro
> when a static function is actually added.

Will fix.

>
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 69d75515ed3f7..8de2a4e5f5de2 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -282,6 +283,20 @@ __printf(2, 3) static void verbose(void *private_data, const char *fmt, ...)
> >       va_end(args);
> >  }
> >
> > +__printf(2, 3) static void warn(void *private_data, const char *fmt, ...)
> > +{
> > +     struct bpf_verifier_env *env = private_data;
> > +     va_list args;
> > +
> > +     if (!bpf_verifier_warn_needed(&env->log))
> > +             return;
> > +
> > +     va_start(args, fmt);
> > +     bpf_verifier_vlog(&env->log, fmt, args);
> > +     va_end(args);
> > +     env->warnings = true;
> > +}
>
> Is this static function called anywhere in this patch?
>
> Unused static functions trigger a -Wunused-function compiler warning, which
> can break builds when CONFIG_WERROR is enabled.

Would just be unnecessary diff noise.

>
> [ ... ]
>
> > @@ -1683,7 +1698,8 @@ static int pop_stack(struct bpf_verifier_env *env, int *prev_insn_idx,
> >               if (err)
> >                       return err;
> >       }
> > -     if (pop_log)
> > +     /* Preserve warning output across branch explorations. */
> > +     if (pop_log && !(env->warnings && bpf_verifier_warn_needed(&env->log)))
> >               bpf_vlog_reset(&env->log, head->log_pos);
> >       if (insn_idx)
> >               *insn_idx = head->insn_idx;
>
> Can this completely disable log resets once a warning is emitted?
>
> If a user requests both standard logging and warning logging, both verbose()
> and warn() will actively write to the log buffer.
>
> If log pruning is permanently disabled after the first warning, will the
> verifier dump the state of every successfully verified instruction into the
> log for all subsequent branch explorations?
>
> Because the log buffer acts as a rotating buffer by default, could this
> flood of output wrap around and overwrite the warning that was meant to be
> preserved?
>
> Would it be better to track the log position of the warning and reset to
> the maximum of head->log_pos and the warning position?

Good point, should probably do this only when log_level == 16.

>
> [ ... ]
>
> > @@ -18803,7 +18819,8 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
> >
> >       ret = do_check(env);
> >  out:
> > -     if (!ret && pop_log)
> > +     if (!ret && pop_log &&
> > +         !(env->warnings && bpf_verifier_warn_needed(&env->log)))
> >               bpf_vlog_reset(&env->log, 0);
> >       free_states(env);
> >       return ret;
>
> Could the same log buffer wrapping issue happen here when log resets are
> disabled after a warning?

Likewise, will fix.

>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1