On Wed, Nov 15, 2017 at 11:13:00AM +0100, Florian Westphal wrote:
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.
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?