From: Florian Westphal <fw@strlen.de>
To: Sebastian Gottschall <s.gottschall@dd-wrt.com>
Cc: Guillaume Nault <g.nault@alphalink.fr>,
Florian Westphal <fw@strlen.de>,
stable@vger.kernel.org, pablo@netfilter.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH 4.9.y] netfilter: nat: Revert "netfilter: nat: convert nat bysrc hash to rhashtable"
Date: Wed, 15 Nov 2017 11:13:00 +0100 [thread overview]
Message-ID: <20171115101300.GR5512@breakpoint.cc> (raw)
In-Reply-To: <66c71431-aa02-b55e-b35a-67e08433c5fc@dd-wrt.com>
Sebastian Gottschall <s.gottschall@dd-wrt.com> wrote:
> the following patch based on the suggestion by Guillaume works good in my
> tests and does not crash anymore
>
> --- nf_nat_core.cᅵᅵᅵᅵᅵᅵ (Revision 33766)
> +++ nf_nat_core.cᅵᅵᅵᅵᅵᅵ (Arbeitskopie)
> @@ -678,16 +678,11 @@
> ᅵ/* No one using conntrack by the time this called. */
> ᅵstatic void nf_nat_cleanup_conntrack(struct nf_conn *ct)
> ᅵ{
> -ᅵᅵᅵᅵᅵᅵ struct nf_conn_nat *nat = nf_ct_ext_find(ct, NF_CT_EXT_NAT);
> -
> -ᅵᅵᅵᅵᅵᅵ if (!nat)
> -ᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵ return;
> -
> -ᅵᅵᅵᅵᅵᅵ NF_CT_ASSERT(ct->status & IPS_SRC_NAT_DONE);
> -
> -ᅵᅵᅵᅵᅵᅵ spin_lock_bh(&nf_nat_lock);
> -ᅵᅵᅵᅵᅵᅵ hlist_del_rcu(&ct->nat_bysource);
> -ᅵᅵᅵᅵᅵᅵ spin_unlock_bh(&nf_nat_lock);
> +ᅵᅵᅵᅵᅵᅵ if (ct->status & IPS_SRC_NAT_DONE) {
> +ᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵ spin_lock_bh(&nf_nat_lock);
> +ᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵ hlist_del_rcu(&ct->nat_bysource);
> +ᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵᅵ spin_unlock_bh(&nf_nat_lock);
> +ᅵᅵᅵᅵᅵᅵ }
> ᅵ}
FWIW I can now reproduce the crash on 4.9-stable + backport.
This crash can be triggered by a simple
iptables -I INPUT 1 -j DROP
on first (new) incoming packet.
Problem is that as not SRC nat transformation is there for
inbound connections such conntrack isn't yet on nat_bysource list
so hlist_del_rcu() gets garbage input.
bug was added in
7c9664351980aaa6a4b8837a314360b3a4ad382a
("netfilter: move nat hlist_head to nf_conn").
... and then 'masked' by the rhashtable conversion (removing non-existing
rhashtable element has no ill effects).
The revert upstream and the 4.13.y one had no such issue because of earlier
change 6e699867f84c0f358fed233fe6162173aca28e04
("netfilter: nat: avoid use of nf_conn_nat extension"), which replaces
if (!nat) condition with the if (ct->status & IPS_SRC_NAT_DONE) test
quoted above.
I will have an updated patch shortly, I think best solution is to just
cherry-pick 6e699867f84c0f358fed233fe6162173aca28e04 in 4.9.y too
and update this backport accordingly.
next prev parent reply other threads:[~2017-11-15 10:13 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-12 20:23 [PATCH 4.9.y] netfilter: nat: Revert "netfilter: nat: convert nat bysrc hash to rhashtable" Florian Westphal
2017-11-13 8:36 ` Greg KH
2017-11-14 7:07 ` Sebastian Gottschall
2017-11-14 11:05 ` Florian Westphal
2017-11-14 11:54 ` Guillaume Nault
2017-11-14 19:30 ` Guillaume Nault
2017-11-14 22:16 ` Sebastian Gottschall
2017-11-14 22:44 ` Sebastian Gottschall
2017-11-15 10:13 ` Florian Westphal [this message]
2017-11-15 14:04 ` Greg Kroah-Hartman
2017-11-15 14:16 ` Pablo Neira Ayuso
2017-11-15 14:51 ` Greg Kroah-Hartman
2017-11-15 22:57 ` Sebastian Gottschall
2017-11-15 14:11 ` Pablo Neira Ayuso
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=20171115101300.GR5512@breakpoint.cc \
--to=fw@strlen.de \
--cc=g.nault@alphalink.fr \
--cc=gregkh@linuxfoundation.org \
--cc=pablo@netfilter.org \
--cc=s.gottschall@dd-wrt.com \
--cc=stable@vger.kernel.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.