From patchwork Thu Jan 10 16:41:54 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pablo Neira Ayuso X-Patchwork-Id: 211086 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 905E42C0223 for ; Fri, 11 Jan 2013 03:42:01 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753448Ab3AJQl7 (ORCPT ); Thu, 10 Jan 2013 11:41:59 -0500 Received: from mail.us.es ([193.147.175.20]:49251 "EHLO mail.us.es" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752427Ab3AJQl6 (ORCPT ); Thu, 10 Jan 2013 11:41:58 -0500 Received: (qmail 29306 invoked from network); 10 Jan 2013 17:41:56 +0100 Received: from unknown (HELO us.es) (192.168.2.12) by us.es with SMTP; 10 Jan 2013 17:41:56 +0100 Received: (qmail 16668 invoked by uid 507); 10 Jan 2013 16:41:56 -0000 X-Qmail-Scanner-Diagnostics: from 127.0.0.1 by antivirus2 (envelope-from , uid 501) with qmail-scanner-2.10 (clamdscan: 0.97.6/16459. spamassassin: 3.3.2. Clear:RC:1(127.0.0.1):SA:0(-99.2/7.5):. Processed in 1.790319 secs); 10 Jan 2013 16:41:56 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on antivirus2 X-Spam-Level: X-Spam-Status: No, score=-99.2 required=7.5 tests=BAYES_50, RP_MATCHES_RCVD, SPF_HELO_FAIL, USER_IN_WHITELIST autolearn=disabled version=3.3.2 X-Envelope-From: pneira@us.es Received: from unknown (HELO antivirus2) (127.0.0.1) by us.es with SMTP; 10 Jan 2013 16:41:55 -0000 Received: from 192.168.1.13 (192.168.1.13) by antivirus2 (F-Secure/fsigk_smtp/407/antivirus2); Thu, 10 Jan 2013 17:41:54 +0100 (CET) X-Virus-Status: clean(F-Secure/fsigk_smtp/407/antivirus2) Received: (qmail 18212 invoked from network); 10 Jan 2013 17:41:54 +0100 Received: from 1984.lsi.us.es (HELO us.es) (1984lsi@150.214.188.80) by us.es with AES128-SHA encrypted SMTP; 10 Jan 2013 17:41:54 +0100 Date: Thu, 10 Jan 2013 17:41:54 +0100 From: Pablo Neira Ayuso To: Gao feng Cc: netfilter-devel@vger.kernel.org, netdev@vger.kernel.org, canqunzhang@gmail.com, kaber@trash.net, ebiederm@xmission.com Subject: Re: [PATCH 01/19] netfilter: move nf_conntrack initialize out of pernet operations Message-ID: <20130110164154.GA5457@1984> References: <1356662206-2260-1-git-send-email-gaofeng@cn.fujitsu.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1356662206-2260-1-git-send-email-gaofeng@cn.fujitsu.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Hi Gao, On Fri, Dec 28, 2012 at 10:36:27AM +0800, Gao feng wrote: > canqun zhang reported a panic BUG,kernel may panic when > unloading nf_conntrack module. > > It's because we reset nf_ct_destroy to NULL when we deal > with init_net,it's too early.Some packets belongs to other > netns still refers to the conntrack.when these packets need > to be freed, kfree_skb will call nf_ct_destroy which is > NULL. > > fix this bug by moving the nf_conntrack initialize and cleanup > codes out of the pernet operations,this job should be done > in module_init/exit.We can't use init_net to identify if > it's the right time. First off, thanks for looking into this. I want to get this fix into 3.8 and -stable but this patch includes a rework whose scope is net-next (upcoming 3.9). The attached patch aims to fix the issue according to your patch description. Once this is in, we can revisit your code refactoring proposal. Let me know. Acked-by: Gao feng From a211bd666fbfe17ae7171a50ad92fedc7b9e19fa Mon Sep 17 00:00:00 2001 From: Pablo Neira Ayuso Date: Thu, 10 Jan 2013 16:12:01 +0100 Subject: [PATCH] netfilter: nf_conntrack: fix BUG_ON while removing nf_conntrack with netns canqun zhang reported that we're hitting BUG_ON in the nf_conntrack_destroy path when calling kfree_skb while rmmod'ing the nf_conntrack module. Currently, the nf_ct_destroy hook is being set to NULL in the destroy path of conntrack.init_net. However, this is a problem since init_net may be destroyed before any other existing netns (we cannot assume any specific ordering while releasing existing netns according to what I read in recent emails). Thanks to Gao feng for initial patch to address this issue. Reported-by: canqun zhang Signed-off-by: Pablo Neira Ayuso --- include/net/netfilter/nf_conntrack_core.h | 2 ++ net/netfilter/nf_conntrack_core.c | 9 +++++---- net/netfilter/nf_conntrack_standalone.c | 1 + 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h index d8f5b9f..e98aeb3 100644 --- a/include/net/netfilter/nf_conntrack_core.h +++ b/include/net/netfilter/nf_conntrack_core.h @@ -31,6 +31,8 @@ extern void nf_conntrack_cleanup(struct net *net); extern int nf_conntrack_proto_init(struct net *net); extern void nf_conntrack_proto_fini(struct net *net); +extern void nf_conntrack_cleanup_end(void); + extern bool nf_ct_get_tuple(const struct sk_buff *skb, unsigned int nhoff, diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c index 016d95e..e4a0c4f 100644 --- a/net/netfilter/nf_conntrack_core.c +++ b/net/netfilter/nf_conntrack_core.c @@ -1376,11 +1376,12 @@ void nf_conntrack_cleanup(struct net *net) synchronize_net(); nf_conntrack_proto_fini(net); nf_conntrack_cleanup_net(net); +} - if (net_eq(net, &init_net)) { - RCU_INIT_POINTER(nf_ct_destroy, NULL); - nf_conntrack_cleanup_init_net(); - } +void nf_conntrack_cleanup_end(void) +{ + RCU_INIT_POINTER(nf_ct_destroy, NULL); + nf_conntrack_cleanup_init_net(); } void *nf_ct_alloc_hashtable(unsigned int *sizep, int nulls) diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c index 363285d..e7185c6 100644 --- a/net/netfilter/nf_conntrack_standalone.c +++ b/net/netfilter/nf_conntrack_standalone.c @@ -575,6 +575,7 @@ static int __init nf_conntrack_standalone_init(void) static void __exit nf_conntrack_standalone_fini(void) { unregister_pernet_subsys(&nf_conntrack_net_ops); + nf_conntrack_cleanup_end(); } module_init(nf_conntrack_standalone_init); -- 1.7.10.4