Hi,
On Tue, Jun 15, 2021 at 06:01:49PM +0200, Florian Westphal wrote:
> Pablo reports following input and output:
> in: icmpv6 id 1
> out: icmpv6 type { echo-request, echo-reply } icmpv6 parameter-problem 65536/16
>
> Reason is that icmp fields overlap, decoding of the correct name requires
> check of the icmpv6 type. This only works for equality tests, for
> instance
>
> in: icmpv6 type echo-request icmpv6 id 1
> will be listed as "icmpv6 id 1" (which is not correct either, since the
> input only matches on echo-request).
>
> with this patch, output of 'icmpv6 id 1' is
> icmpv6 type { echo-request, echo-reply } icmpv6 id 1
>
> The second problem, the removal of a single check (request OR reply),
> is resolved in the followup patch.
>
> Signed-off-by: Florian Westphal <fw@xxxxxxxxx>
Eric reported a testcase in which this patch seems to cause a segfault
(bisected). The test is as simple as:
| nft -f - <<EOF
| add table inet firewalld_check_rule_index
| add chain inet firewalld_check_rule_index foobar { type filter hook input priority 0 ; }
| add rule inet firewalld_check_rule_index foobar tcp dport 1234 accept
| add rule inet firewalld_check_rule_index foobar accept
| insert rule inet firewalld_check_rule_index foobar index 1 udp dport 4321 accept
| EOF
But a ruleset is in place at this time. Also, I can't reproduce it on my
own machine but only on Eric's VM for testing.
[...]
> static void payload_match_postprocess(struct rule_pp_ctx *ctx,
> struct expr *expr,
> struct expr *payload)
> @@ -1883,6 +1932,19 @@ static void payload_match_postprocess(struct rule_pp_ctx *ctx,
> if (expr->right->etype == EXPR_VALUE) {
> payload_match_expand(ctx, expr, payload);
> break;
> + } else if (expr->right->etype == EXPR_SET_REF) {
> + struct set *set = expr->right->set;
> +
> + if (set_is_anonymous(set->flags) &&
> + !list_empty(&set->init->expressions)) {
According to GDB, set->init is NULL here.
I am not familiar with recent changes in cache code, maybe there's the
actual culprit: Debug printf in cache_init_objects() states flags
variable is 0x4000005f, i.e. NFT_CACHE_SETELEM_BIT is not set.
I am not sure if caching is incomplete and we need that bit or if the
above code should expect sets with missing elements and therefore check
'set->init != NULL' before accessing expressions field.
Cheers, Phil
