From patchwork Fri Jul 29 16:19:04 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michal Kubecek X-Patchwork-Id: 654165 X-Patchwork-Delegate: pablo@netfilter.org 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 3s1DTJ4R1Wz9t0p for ; Sat, 30 Jul 2016 02:19:48 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752510AbcG2QTJ (ORCPT ); Fri, 29 Jul 2016 12:19:09 -0400 Received: from mx2.suse.de ([195.135.220.15]:40375 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751563AbcG2QTH (ORCPT ); Fri, 29 Jul 2016 12:19:07 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 8FD48ABBB; Fri, 29 Jul 2016 16:19:04 +0000 (UTC) Received: by unicorn.suse.cz (Postfix, from userid 1000) id 4F00BA0BD9; Fri, 29 Jul 2016 18:19:04 +0200 (CEST) In-Reply-To: <20160729150033.E0250A0BD9@unicorn.suse.cz> References: <20160729150033.E0250A0BD9@unicorn.suse.cz> From: Michal Kubecek Subject: [PATCH RESEND nf] netfilter: avoid a race between nf_register_hook() and cleanup_net() To: Pablo Neira Ayuso Cc: Patrick McHardy , Jozsef Kadlecsik , netfilter-devel@vger.kernel.org, coreteam@netfilter.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, "Eric W. Biederman" Message-Id: <20160729161904.4F00BA0BD9@unicorn.suse.cz> Date: Fri, 29 Jul 2016 18:19:04 +0200 (CEST) Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org There is a race condition between nf_{,un}register_hook() and cleanup_net() which can either trigger WARN check or cause a memory leak. The scenario is like this (2a and 2b are alternatives): 1. cleanup_net() removes one or more struct net from net_namespace_list 2a. nf_register_hook() adds per-netns hooks to all netns (but not those removed in step 1) and adds the hook to global nf_hook_list 2b. nf_unregister_hook() deletes per-netns hooks from all netns (but not those removed in step 1) and removes the hook from nf_hook_list 3. cleanup_net() calls pernet subsystem exit functions for netns being removed; one of them is netfilter_net_exit() which (among others) calls nf_unregister_net_hook() to unregister per-netns hooks for all hooks in nf_hook_list. In case (a), per-netns hooks are never added as the namespace was already invisible to for_each_net() in step 2a but an attempt to remove them in step 3 (the hook is already in nf_hook_list) triggers a WARN check in nf_unregister_net_hook() (no real harm done, however). In case (b), the per-netns hook is removed neither in step 2b (netns is already invisible to for_each_net()) nor in step 3 (the hook is already removed from nf_hook_list), causing a memory leak. Prevent the race by protecting the for_each_net() loop in nf_{,un}register_hook() (also) by net_mutex. There is already a precendens for this in rtnl_link_unregister() which addresses similar race. Fixes: 085db2c04557 ("netfilter: Per network namespace netfilter hooks.") Signed-off-by: Michal Kubecek Reviewed-by: "Eric W. Biederman" --- net/netfilter/core.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/net/netfilter/core.c b/net/netfilter/core.c index f39276d1c2d7..860978c9f82e 100644 --- a/net/netfilter/core.c +++ b/net/netfilter/core.c @@ -193,6 +193,8 @@ int nf_register_hook(struct nf_hook_ops *reg) struct net *net, *last; int ret; + /* prevent race with cleanup_net() */ + mutex_lock(&net_mutex); rtnl_lock(); for_each_net(net) { ret = nf_register_net_hook(net, reg); @@ -201,6 +203,7 @@ int nf_register_hook(struct nf_hook_ops *reg) } list_add_tail(®->list, &nf_hook_list); rtnl_unlock(); + mutex_unlock(&net_mutex); return 0; rollback: @@ -211,6 +214,7 @@ rollback: nf_unregister_net_hook(net, reg); } rtnl_unlock(); + mutex_unlock(&net_mutex); return ret; } EXPORT_SYMBOL(nf_register_hook); @@ -219,11 +223,14 @@ void nf_unregister_hook(struct nf_hook_ops *reg) { struct net *net; + /* prevent race with cleanup_net() */ + mutex_lock(&net_mutex); rtnl_lock(); list_del(®->list); for_each_net(net) nf_unregister_net_hook(net, reg); rtnl_unlock(); + mutex_unlock(&net_mutex); } EXPORT_SYMBOL(nf_unregister_hook);