Re: [PATCH iptables] extensions: libebt_stp: fix range checking

[Phil Sutter <phil@nwl.cc>]

On Wed, Jan 24, 2024 at 04:42:16PM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > While you correctly hate the game instead of its player, you probably
> > hate the wrong game: The code above indeed is confusing. Maybe one
> > should move that monotonicity check into libxtables which should
> > simplify it quite a bit. I'll have a look. :)
> 
> Something IS broken.  Still not working on FC 39 test machine
> even after fresh clone.
> 
> On a "working" VM:
> export XTABLES_LIBDIR=$(pwd)/extensions
> iptables/xtables-nft-multi ebtables -A INPUT --stp-root-cost 1
> have 1 32765
> 
> @@ -150,7 +151,9 @@ static void brstp_parse(struct xt_option_call *cb)
>                 RANGE_ASSIGN("root-prio", root_prio, cb->val.u16_range);
>                 break;
>         case O_RCOST:
> +               fprintf(stderr, "have %u %u\n", cb->val.u32_range[0], cb->val.u32_range[1]);
> 
> I can't even figure out where the correct max value is supposed to be set.
> 
> Varying the input:
> 
> xtables-nft-multi ebtables -A INPUT --stp-root-cost 1
> have 1 32764
> 
> Looks to me as if the upper value is undefined.
> 
> Other users of *RC versions handle it in .parse, e.g. libxt_length.
> No idea how this is working.

In xtopt_parse_mint(), there is:

| const uintmax_t lmax = xtopt_max_by_type(entry->type);
| [...]
| if (*arg == '\0' || *arg == sep) {
|         /* Default range components when field not spec'd. */
|         end = (char *)arg;
|         value = (cb->nvals == 1) ? lmax : 0;

But that branch appears to be dead code. So this is indeed a bug and a
specific build may or may not hit it as your experience shows. I'll see
how xtopt_parse_mint() can be fixed.

The reason why extensions may sanitize the values is because kernel code
may expect upper==lower or upper==MAX if not given.

Cheers, Phil