From: Florian Westphal <fw@strlen.de>
To: Phil Sutter <phil@nwl.cc>, Florian Westphal <fw@strlen.de>,
netfilter-devel@vger.kernel.org,
Pablo Neira Ayuso <pablo@netfilter.org>, Eric Garver <e@erig.me>
Subject: Re: [PATCH nft v2 1/3] netlink_delinearize: add missing icmp id/sequence support
Date: Wed, 30 Jun 2021 17:34:25 +0200 [thread overview]
Message-ID: <20210630153425.GD18022@breakpoint.cc> (raw)
In-Reply-To: <20210630151319.GZ3673@orbyte.nwl.cc>
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.
next prev parent reply other threads:[~2021-06-30 15:34 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-15 16:01 [PATCH nft v2 0/3] fix icmpv6 id dependeny handling Florian Westphal
2021-06-15 16:01 ` [PATCH nft v2 1/3] netlink_delinearize: add missing icmp id/sequence support Florian Westphal
2021-06-30 15:13 ` Phil Sutter
2021-06-30 15:34 ` Florian Westphal [this message]
2021-06-30 15:58 ` Florian Westphal
2021-06-30 17:12 ` Phil Sutter
2021-06-15 16:01 ` [PATCH nft v2 2/3] payload: do not remove icmp echo dependency Florian Westphal
2021-06-15 16:01 ` [PATCH nft v2 3/3] tests: add a icmp-reply only and icmpv6 id test cases Florian Westphal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210630153425.GD18022@breakpoint.cc \
--to=fw@strlen.de \
--cc=e@erig.me \
--cc=netfilter-devel@vger.kernel.org \
--cc=pablo@netfilter.org \
--cc=phil@nwl.cc \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).