stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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.

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