Re: [PATCH] security: remove BUG_ON in security_skb_classify_flow

[Stephen Smalley]

On Fri, Apr 10, 2026 at 7:36 PM Serge E. Hallyn <serge@xxxxxxxxxx> wrote:
>
> On Fri, Apr 10, 2026 at 09:56:22AM +0800, Jiayuan Chen wrote:
> >
> > On 4/10/26 8:58 AM, Serge E. Hallyn wrote:
> > > On Wed, Apr 08, 2026 at 07:42:57PM +0800, Jiayuan Chen wrote:
> > > > A BPF program attached to the xfrm_decode_session hook can return a
> > > > non-zero value, which causes BUG_ON(rc) in security_skb_classify_flow()
> > > > to trigger a kernel panic.
> > > It would seem worth it to have pointed at the previous discussion at
> > >
> > > https://lore.kernel.org/all/CAEjxPJ5aA01in+Z1yLF1cwe-3uqL_E8SKGK4J294D5eRG5__5Q@xxxxxxxxxxxxxx/
> > >
> > > Based on that, I guess this is probably ok, but still,
> > >
> > > > Remove the BUG_ON and change the return type from void to int, so that
> > > > callers can optionally handle the error.
> > > but you don't have the existing callers handling the error.  It's
> > > conceivable they won't care, but it's also possible that they were
> > > counting on a BUG_ON in that case.
> > >
> > > What *should* callers (icmp_reply, etc) do if an error code is
> > > returned?  Should they ignore it?  In that case, would it be
> > > better to change security_skb_classify_flow() to return void?
> > >
> > Thanks for your pointer.
> >
> > So I think Feng's patch is sufficient and can by applied ?
>
> Well, selinux_xfrm_decode_session() calls selinux_xfrm_skb_sid_ingress()
> which *can* return -EINVAL.
>
> So I'd like to know, what is supposed to happen in that case?
>
> Stephen, do you know?  Is it safe for callers to ignore this?

I'm in favor of dropping the BUG_ON() from
security_skb_classify_flow() and just make it return void, ignoring
any non-zero return from xfrm_decode_session.
A slightly cleaner approach would be to introduce a separate LSM hook
for skb_classify_flow() that returns void rather than calling
xfrm_decode_session() from security_skb_classify_flow(); then any bug
handling can be done in the individual security module.