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@xxxxxxxxxxxxx> wrote: > > > > On 5/9/25 12:31 AM, Alexei Starovoitov wrote: > > > On Thu, May 8, 2025 at 1:40 PM Paul Chaignon <paul.chaignon@xxxxxxxxx> 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.
