From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.us.es ([193.147.175.20]:33174 "EHLO mail.us.es" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757950AbdKOOLP (ORCPT ); Wed, 15 Nov 2017 09:11:15 -0500 Received: from antivirus1-rhel7.int (unknown [192.168.2.11]) by mail.us.es (Postfix) with ESMTP id 9DB26210565 for ; Wed, 15 Nov 2017 15:11:13 +0100 (CET) Received: from antivirus1-rhel7.int (localhost [127.0.0.1]) by antivirus1-rhel7.int (Postfix) with ESMTP id 8AEB1DA86E for ; Wed, 15 Nov 2017 15:11:13 +0100 (CET) Date: Wed, 15 Nov 2017 15:11:08 +0100 From: Pablo Neira Ayuso To: Florian Westphal Cc: Sebastian Gottschall , Guillaume Nault , stable@vger.kernel.org, Greg Kroah-Hartman Subject: Re: [PATCH 4.9.y] netfilter: nat: Revert "netfilter: nat: convert nat bysrc hash to rhashtable" Message-ID: <20171115141108.GA1064@salvia> References: <20171112202347.14762-1-fw@strlen.de> <27ad57ea-74dc-adaf-de87-65ac43fc0e26@dd-wrt.com> <20171114110546.GA32514@breakpoint.cc> <20171114193011.ju5dg623omnrniuf@alphalink.fr> <66c71431-aa02-b55e-b35a-67e08433c5fc@dd-wrt.com> <20171115101300.GR5512@breakpoint.cc> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20171115101300.GR5512@breakpoint.cc> Sender: stable-owner@vger.kernel.org List-ID: On Wed, Nov 15, 2017 at 11:13:00AM +0100, Florian Westphal wrote: > Sebastian Gottschall 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. That's fine. Let's just make sure we get them both patches in together in the same go. So I think it's better if we just keep back this cherry-pick until we have the necessary backport in place. OK?