Re: [PATCH ipsec-next] xfrm: Use regular error handling instead of BUG_ON() in the netlink API.

[Sabrina Dubroca <sd@queasysnail.net>]

2026-05-07, 06:21:57 +0100, Antony Antony wrote:
> wHi Steffen,
> 
> Thanks Steffen, I was hit by this in the new XFRM_MIGRATE_STATE I am adding.
> I am glad to see we are addressing this.
> 
> On Wed, May 06, 2026 at 06:08:55PM +0200, Steffen Klassert via Devel wrote:
> > The xfrm netlink API uses BUG_ON() on failures since it exists.
> > However all these error are uncritical and can be handled
> > with regular error handling. This fixes machine crashes
> > in situations where an emergency break is not needed.
> 
> While BUG_ON is an extreme measure for a recoverable netlink error, it does
> have diagnostic value: it leaves a stack trace. The patch trades
> a crash + stack trace for a silent error return, which loses observability.
> 
> Would you consider using WARN_ONCE instead of a bare if (err < 0)?
> 
> -     BUG_ON(err < 0);
> +     if (WARN_ONCE(err < 0, "xfrm: build_spdinfo failed: %d\n", err)) {
> +         kfree_skb(r_skb);
> +         return err;
> +     }

OTOH we already have a bunch of functions doing something similar
without using BUG_ON/WARN_ON, so at least with this patch it becomes
consistent.

xfrm_notify_userpolicy
xfrm_get_default
xfrm_get_ae
xfrm_exp_state_notify
xfrm_notify_sa_flush
xfrm_notify_sa
xfrm_notify_policy
xfrm_notify_policy_flush


(I'm looking into generic ways to avoid this split getsize/fill that
always becomes inconsistent in areas where new attributes are added
frequently, but nothing to share yet)

> Something like the above would preserve the "shouldn't happen" signal with a 
> stack trace on first occurrence, without panicking the machine.
> Or are there better signaling  styles in Kernel?

Maybe DEBUG_NET_WARN_ON_ONCE so that only developers see those messages.

-- 
Sabrina