Re: [PATCH nft] evalute: don't BUG on unexpected base datatype

[Pablo Neira Ayuso <pablo@netfilter.org>]

Hi Florian,

On Mon, Jun 16, 2025 at 11:16:18AM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > >  I wonder if we should just replace all BUGs in evaluate.c
> > > >  with expr_error() calls, it avoids constant whack-a-mole.
> > 
> > I think that can help uncover bugs, or are those json induced bugs?
> 
> This one is bison, see included bogon input.

For this particular case I think it makes sense to turn it into error.

Nitpick for this patch: I'd suggest this error:

+               return expr_error(ctx->msgs, *expr, "Unexpected datatype %s",
+                                 (*expr)->dtype->name);

Which results in:

[...]
ruleset:3:61-67: Error: Unexpected datatype verdict
               type ipv4_addr . inet_service : ipv4_addr .  verdict
                                                            ^^^^^^^
> I meant in general.

It's not so straight forward, I have quickly browsed the existing
BUG() calls and, unlike this one, they are mostly trapping nonsense,
I guess we can find a way to keep them around while not invoking
assert().

> How do these BUGs help at all?

Maybe you can turn them into error record adding "BUG:" prefix if
you prefer, I guess that will not crash a daemon process which is your
concern?

ie. add expr_bug() to highlight an input is triggering a buggy internal
state so this can still be reported without crashing the daemon
process?