Re: [nft PATCH] fib: Fix for existence check on Big Endian

[Phil Sutter <phil@nwl.cc>]

Hi Pablo,

On Tue, Sep 09, 2025 at 11:34:00PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Sep 09, 2025 at 10:49:48PM +0200, Phil Sutter wrote:
> > Adjust the expression size to 1B so cmp expression value is correct.
> > Without this, the rule 'fib saddr . iif check exists' generates
> > following byte code on BE:
> > 
> > |  [ fib saddr . iif oif present => reg 1 ]
> > |  [ cmp eq reg 1 0x00000001 ]
> > 
> > Though with NFTA_FIB_F_PRESENT flag set, nft_fib.ko writes to the first
> > byte of reg 1 only (using nft_reg_store8()). With this patch in place,
> > byte code is correct:
> > 
> > |  [ fib saddr . iif oif present => reg 1 ]
> > |  [ cmp eq reg 1 0x01000000 ]
> 
> Is this a generic issue of boolean that is using 1 bit?
> 
> const struct datatype boolean_type = {
>         .type           = TYPE_BOOLEAN,
>         .name           = "boolean",
>         .desc           = "boolean type",
>         .size           = 1,

Maybe, yes: I compared fib existence checks to exthdr ones in order to
find the bug. With exthdr, we know in parser already that it is an
existence check (see exthdr_exists_expr rule in parser_bison.y). If so,
exthdr expression is allocated with type 1 which is (assumed to be) the
NEXTHDR field in all extension headers. This field has
inet_protocol_type, which is size 8b.

Via expr_ctx::len, RHS will then be adjusted to 8b size (see 'expr->len =
masklen' in expr_evaluate_integer()).

IIRC, LHS defines the RHS size in relationals. I am not sure if we may
sanely reverse this rule if RHS is a boolean_type.

Cheers, Phil