From patchwork Fri Feb 24 22:38:23 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pablo Neira Ayuso X-Patchwork-Id: 142991 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 0BEA1B6FAA for ; Sat, 25 Feb 2012 09:38:44 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932354Ab2BXWim (ORCPT ); Fri, 24 Feb 2012 17:38:42 -0500 Received: from mail.us.es ([193.147.175.20]:37271 "EHLO mail.us.es" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755583Ab2BXWik (ORCPT ); Fri, 24 Feb 2012 17:38:40 -0500 Received: (qmail 10886 invoked from network); 24 Feb 2012 23:38:38 +0100 Received: from unknown (HELO us.es) (192.168.2.13) by us.es with SMTP; 24 Feb 2012 23:38:38 +0100 Received: (qmail 26498 invoked by uid 507); 24 Feb 2012 22:38:37 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on antivirus3 X-Spam-Level: X-Spam-Status: No, score=-97.8 required=7.5 tests=BAYES_50,KHOP_DYNAMIC, RCVD_IN_PBL, RCVD_IN_SORBS_DUL, RDNS_DYNAMIC, T_FRT_CONTACT, USER_IN_WHITELIST autolearn=disabled version=3.3.1 Received: from 127.0.0.1 by antivirus3 (envelope-from , uid 501) with qmail-scanner-2.08 (clamdscan: 0.97.3/14525. Clear:RC:1(127.0.0.1):. Processed in 0.025012 secs); 24 Feb 2012 22:38:37 -0000 Received: from unknown (HELO antivirus3) (127.0.0.1) by us.es with SMTP; 24 Feb 2012 22:38:37 -0000 Received: from 192.168.1.13 (192.168.1.13) by antivirus3 (F-Secure/fsigk_smtp/407/antivirus3); Fri, 24 Feb 2012 23:38:37 +0100 (CET) X-Virus-Status: clean(F-Secure/fsigk_smtp/407/antivirus3) Received: (qmail 2720 invoked from network); 24 Feb 2012 23:38:37 +0100 Received: from 192.181.22.95.dynamic.jazztel.es (HELO localhost.localdomain) (pneira@us.es@95.22.181.192) by us.es with SMTP; 24 Feb 2012 23:38:37 +0100 From: pablo@netfilter.org To: netdev@vger.kernel.org Cc: netfilter-devel@vger.kernel.org, davem@davemloft.net Subject: [PATCH 1/2] Revert "netfilter: ctnetlink: fix soft lockup when netlink adds new entries" Date: Fri, 24 Feb 2012 23:38:23 +0100 Message-Id: <1330123104-3953-2-git-send-email-pablo@netfilter.org> X-Mailer: git-send-email 1.7.7.3 In-Reply-To: <1330123104-3953-1-git-send-email-pablo@netfilter.org> References: <1330123104-3953-1-git-send-email-pablo@netfilter.org> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Pablo Neira Ayuso This reverts commit af14cca162ddcdea017b648c21b9b091e4bf1fa4. This patch contains a race condition between packets and ctnetlink in the conntrack addition. A new patch to fix this issue follows up. Signed-off-by: Pablo Neira Ayuso --- net/netfilter/nf_conntrack_netlink.c | 43 +++++++++++++++++++++------------ 1 files changed, 27 insertions(+), 16 deletions(-) diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c index cc70517..9307b03 100644 --- a/net/netfilter/nf_conntrack_netlink.c +++ b/net/netfilter/nf_conntrack_netlink.c @@ -1367,12 +1367,15 @@ ctnetlink_create_conntrack(struct net *net, u16 zone, nf_ct_protonum(ct)); if (helper == NULL) { rcu_read_unlock(); + spin_unlock_bh(&nf_conntrack_lock); #ifdef CONFIG_MODULES if (request_module("nfct-helper-%s", helpname) < 0) { + spin_lock_bh(&nf_conntrack_lock); err = -EOPNOTSUPP; goto err1; } + spin_lock_bh(&nf_conntrack_lock); rcu_read_lock(); helper = __nf_conntrack_helper_find(helpname, nf_ct_l3num(ct), @@ -1466,10 +1469,7 @@ ctnetlink_create_conntrack(struct net *net, u16 zone, tstamp->start = ktime_to_ns(ktime_get_real()); add_timer(&ct->timeout); - spin_lock_bh(&nf_conntrack_lock); nf_conntrack_hash_insert(ct); - nf_conntrack_get(&ct->ct_general); - spin_unlock_bh(&nf_conntrack_lock); rcu_read_unlock(); return ct; @@ -1490,7 +1490,6 @@ ctnetlink_new_conntrack(struct sock *ctnl, struct sk_buff *skb, struct nf_conntrack_tuple otuple, rtuple; struct nf_conntrack_tuple_hash *h = NULL; struct nfgenmsg *nfmsg = nlmsg_data(nlh); - struct nf_conn *ct; u_int8_t u3 = nfmsg->nfgen_family; u16 zone; int err; @@ -1513,22 +1512,25 @@ ctnetlink_new_conntrack(struct sock *ctnl, struct sk_buff *skb, spin_lock_bh(&nf_conntrack_lock); if (cda[CTA_TUPLE_ORIG]) - h = nf_conntrack_find_get(net, zone, &otuple); + h = __nf_conntrack_find(net, zone, &otuple); else if (cda[CTA_TUPLE_REPLY]) - h = nf_conntrack_find_get(net, zone, &rtuple); - spin_unlock_bh(&nf_conntrack_lock); + h = __nf_conntrack_find(net, zone, &rtuple); if (h == NULL) { err = -ENOENT; if (nlh->nlmsg_flags & NLM_F_CREATE) { + struct nf_conn *ct; enum ip_conntrack_events events; ct = ctnetlink_create_conntrack(net, zone, cda, &otuple, &rtuple, u3); - if (IS_ERR(ct)) - return PTR_ERR(ct); - + if (IS_ERR(ct)) { + err = PTR_ERR(ct); + goto out_unlock; + } err = 0; + nf_conntrack_get(&ct->ct_general); + spin_unlock_bh(&nf_conntrack_lock); if (test_bit(IPS_EXPECTED_BIT, &ct->status)) events = IPCT_RELATED; else @@ -1543,19 +1545,23 @@ ctnetlink_new_conntrack(struct sock *ctnl, struct sk_buff *skb, ct, NETLINK_CB(skb).pid, nlmsg_report(nlh)); nf_ct_put(ct); - } + } else + spin_unlock_bh(&nf_conntrack_lock); return err; } /* implicit 'else' */ + /* We manipulate the conntrack inside the global conntrack table lock, + * so there's no need to increase the refcount */ err = -EEXIST; - ct = nf_ct_tuplehash_to_ctrack(h); if (!(nlh->nlmsg_flags & NLM_F_EXCL)) { - spin_lock_bh(&nf_conntrack_lock); + struct nf_conn *ct = nf_ct_tuplehash_to_ctrack(h); + err = ctnetlink_change_conntrack(ct, cda); - spin_unlock_bh(&nf_conntrack_lock); if (err == 0) { + nf_conntrack_get(&ct->ct_general); + spin_unlock_bh(&nf_conntrack_lock); nf_conntrack_eventmask_report((1 << IPCT_REPLY) | (1 << IPCT_ASSURED) | (1 << IPCT_HELPER) | @@ -1564,10 +1570,15 @@ ctnetlink_new_conntrack(struct sock *ctnl, struct sk_buff *skb, (1 << IPCT_MARK), ct, NETLINK_CB(skb).pid, nlmsg_report(nlh)); - } + nf_ct_put(ct); + } else + spin_unlock_bh(&nf_conntrack_lock); + + return err; } - nf_ct_put(ct); +out_unlock: + spin_unlock_bh(&nf_conntrack_lock); return err; }