Re: [PATCH nft v2 1/3] netlink_delinearize: add missing icmp id/sequence support

[Florian Westphal <fw@strlen.de>]

Phil Sutter <phil@nwl.cc> wrote:
> > 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@strlen.de>
> 
> 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.

You need to add a ruleset with
icmp id 42

then it will crash.  adding 'list ruleset' before EOF avoids it,
because cache gets populated.

> 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.

I think correct fix is to check set->init, we don't need to do
postprocessing if its not going to be printed.