Re: [PATCH bpf-next] bpf: Always WARN_ONCE on verifier bugs

[Paul Chaignon <paul.chaignon@gmail.com>]

On Fri, May 09, 2025 at 10:58:00AM -0700, Alexei Starovoitov wrote:
> On Fri, May 9, 2025 at 1:26 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> > On 5/9/25 12:31 AM, Alexei Starovoitov wrote:
> > > On Thu, May 8, 2025 at 1:40 PM 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. 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.
> > >
> > > No. We cannot do it.
> > > WARN_ONCE is for kernel level issues.
> > > In some configs use panic_on_warn=1 too.
> > > Whereas a verifier bug is contained within a verifier.
> > > It will not bring the kernel down.
> >
> > Agree.
> >
> > > We should remove most of the existing WARN_ONCE instead.
> > > Potentially replace them with pr_info_once().
> >
> > Just a thought, maybe one potential avenue could be to have an equivalent of
> > CONFIG_DEBUG_NET which we make a hard dependency of CONFIG_DEBUG_KERNEL so that
> > /noone/ enables this anywhere in production, and then fuzzers could use it
> > to their advantage. The default case for a DEBUG_BPF_WARN_ONCE would then fall
> > to BUILD_BUG_ON_INVALID() which lets compiler check validity but not generate
> > code.
> 
> Good idea.
> but I'm not a fan of the new kconfig.
> I'd rather combine some of them.
> Like make CONFIG_BPF_JIT depend on CONFIG_BPF_SYSCALL.
> That would remove a ton of empty static inline helpers.
> 
> For this case doing WARN_ON only when IS_ENABLED(CONFIG_DEBUG_KERNEL)
> should be fine.
> 
> I guess we can introduce BPF_WARN_ONCE() and friends family of macros
> and use them everywhere in kernel/bpf/ where the warning is not
> harmful to the kernel ?

Thanks for the reviews! I've sent a v2 that introduces BPF_WARN_ONCE and
uses it in verifier_bug() to emit a warning iff CONFIG_DEBUG_KERNEL is
enabled.

I've refrained from replacing all WARN_ONCE at once as I'm not sure I
have all the context yet to know when BPF_WARN_ONCE is more appropriate.
That said, we should be able to use it for other cases and introduce
sibling macros as we go.