From patchwork Sun Sep 3 14:31:04 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taehee Yoo X-Patchwork-Id: 809302 X-Patchwork-Delegate: pablo@netfilter.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="sag7o1s0"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3xlb4x2dMPz9s7v for ; Mon, 4 Sep 2017 00:31:13 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752922AbdICObM (ORCPT ); Sun, 3 Sep 2017 10:31:12 -0400 Received: from mail-pg0-f65.google.com ([74.125.83.65]:38263 "EHLO mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752913AbdICObL (ORCPT ); Sun, 3 Sep 2017 10:31:11 -0400 Received: by mail-pg0-f65.google.com with SMTP id t3so2964287pgt.5 for ; Sun, 03 Sep 2017 07:31:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=/g3APXRmC7QMqrRlJhn0q+2YkI+UxRzXwxmFJDa+gSA=; b=sag7o1s0l+/xmheQGUCwBR+a+ru+9FNDgsShEOOY6lYNb0+D1sMJ3exx2jlLpPSV8F L1HK/BddcZGXU8HVaqugAo0sur6lzIdBo+YW1F7xfcZrIs2Y78UflyUiBveJjW94Irws qA5Bk4MmRucdiPnhKSpOCSFHDCJY4JVqGGofjC61cHXb4OY42R3xAIva+qZOusi8x9/u DbNw2072NjqT5KpB8yOTmMMLk8cENd+EFtYvJ+tvaNlql+rY9ziCryTMt1BiY/yMFeVk /VaLbKvsows4zN0E2iO6zHdtnmA7XH8uO6u27wiCxixVRJDXF5AYKM309JKqCXrRS5BH E50A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=/g3APXRmC7QMqrRlJhn0q+2YkI+UxRzXwxmFJDa+gSA=; b=XNiCFAujfPITd1BW0jUSnsBWJ1kmUZRiiQqTXC92MSvJrbhjHQ4ZgUp0UyzZAQOk0r 5ffQflBkBLwtDm5GY42xZLnH+4ZgnoUffrfQGIfb8s8U6XvFPIiNAivmZcHemY9Wsios K4wzuHr+S9GYcqkbZ2gSaqtMgITOP4XmYo6U4DDww0iAQs2eLosaRJdUqgwsKEjpPN4D mDG/GRy1NkzJbZu+evpM8aiLyr4lDrPYZiGHc5P/sXIVHLI/naJ08OEjaFJauqFAzZ+K /aGlqZ0cAR5qi1XHxwWaeIc9TqAu/vFv4S2p+GLDHqJKFKwN5RrZQnFU7LLme1tt08T1 SVDg== X-Gm-Message-State: AHPjjUgAd6EuggPbqLEaKqzIT0xHvC9VTs8YR/kikU/YxyNmXz0pFT7A AOUdw91wGFLeSg== X-Google-Smtp-Source: ADKCNb7KbQm2r0CBtc0OEEAgGCgX7OkVh8BEkmERovvvqJoiL+0YVIvibhqghJV3ttkF93bPIySQ+w== X-Received: by 10.99.122.69 with SMTP id j5mr9346850pgn.12.1504449071138; Sun, 03 Sep 2017 07:31:11 -0700 (PDT) Received: from ap-To-be-filled-by-O-E-M.8.8.8.8 ([222.98.178.163]) by smtp.gmail.com with ESMTPSA id l22sm7240047pfg.175.2017.09.03.07.31.09 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 03 Sep 2017 07:31:10 -0700 (PDT) From: Taehee Yoo To: pablo@netfilter.org, netfilter-devel@vger.kernel.org Cc: ap420073@gmail.com Subject: [PATCH] netfilter: ipt_CLUSTERIP: Fix potential deadlock when CLUSTERIP target is inserted Date: Sun, 3 Sep 2017 23:31:04 +0900 Message-Id: <20170903143104.25994-1-ap420073@gmail.com> X-Mailer: git-send-email 2.9.3 Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org When ipt_CLUSTERIP target is inserted, lockdep warns about possible DEADLOCK situation. to avoid deadlock situation register_netdevice_notifier() should be called by only init routine. reproduce command is : # iptables -A INPUT -p tcp -i enp3s0 -d 192.168.0.5 --dport 80 \ -j CLUSTERIP --new --hashmode sourceip \ --clustermac 01:00:5e:00:00:20 --total-nodes 2 --local-node 1 warning message is : [ 148.751110] WARNING: possible circular locking dependency detected [ 148.758037] 4.13.0-rc1+ #71 Not tainted [ 148.762334] ------------------------------------------------------ [ ... ] the existing dependency chain (in reverse order) is: [ 148.816203] -> #1 (sk_lock-AF_INET){+.+.+.}: [ 148.822686] lock_acquire+0x190/0x370 [ 148.827401] lock_sock_nested+0xb8/0x100 [ 148.832405] do_ip_setsockopt.isra.16+0x140/0x24f0 [ 148.838380] ip_setsockopt+0x34/0xb0 [ 148.842988] udp_setsockopt+0x1b/0x30 [ 148.847692] sock_common_setsockopt+0x78/0xf0 [ 148.853182] SyS_setsockopt+0x11c/0x220 [ 148.858089] do_syscall_64+0x187/0x410 [ 148.862901] return_from_SYSCALL_64+0x0/0x7a [ 148.868289] -> #0 (rtnl_mutex){+.+.+.}: [ 148.874303] __lock_acquire+0x4114/0x47c0 [ 148.879405] lock_acquire+0x190/0x370 [ 148.884109] __mutex_lock+0xef/0x1460 [ 148.888820] mutex_lock_nested+0x1b/0x20 [ 148.893824] rtnl_lock+0x17/0x20 [ 148.898052] register_netdevice_notifier+0x6f/0x4f0 [ 148.904127] clusterip_tg_check+0xbf0/0x13e0 [ 148.909519] xt_check_target+0x1f5/0x6c0 [ 148.914525] find_check_entry.isra.7+0x62f/0x960 [ 148.920308] translate_table+0xcf2/0x1830 [ 148.925410] do_ipt_set_ctl+0x1ff/0x3a0 [ 148.930320] nf_setsockopt+0x61/0xc0 [ 148.934933] ip_setsockopt+0x82/0xb0 [ 148.939548] raw_setsockopt+0x73/0xa0 [ 148.944260] sock_common_setsockopt+0x78/0xf0 [ 148.949749] SyS_setsockopt+0x11c/0x220 [ 148.954658] entry_SYSCALL_64_fastpath+0x1c/0xb1 [ 148.960435] other info that might help us debug this: [ 148.969459] Possible unsafe locking scenario: [ 148.976124] CPU0 CPU1 [ 148.981218] ---- ---- [ 148.986312] lock(sk_lock-AF_INET); [ 148.990343] lock(rtnl_mutex); [ 148.996708] lock(sk_lock-AF_INET); [ 149.003559] lock(rtnl_mutex); [ 149.007103] *** DEADLOCK *** [ ... ] Signed-off-by: Taehee Yoo --- net/ipv4/netfilter/ipt_CLUSTERIP.c | 70 +++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/net/ipv4/netfilter/ipt_CLUSTERIP.c b/net/ipv4/netfilter/ipt_CLUSTERIP.c index 6637e8b..c31f188 100644 --- a/net/ipv4/netfilter/ipt_CLUSTERIP.c +++ b/net/ipv4/netfilter/ipt_CLUSTERIP.c @@ -59,7 +59,6 @@ struct clusterip_config { struct rcu_head rcu; char ifname[IFNAMSIZ]; /* device ifname */ - struct notifier_block notifier; /* refresh c->ifindex in it */ }; #ifdef CONFIG_PROC_FS @@ -73,6 +72,7 @@ struct clusterip_net { /* lock protects the configs list */ spinlock_t lock; + struct notifier_block notifier; #ifdef CONFIG_PROC_FS struct proc_dir_entry *procdir; #endif @@ -111,8 +111,6 @@ clusterip_config_entry_put(struct net *net, struct clusterip_config *c) spin_unlock(&cn->lock); local_bh_enable(); - unregister_netdevice_notifier(&c->notifier); - /* In case anyone still accesses the file, the open/close * functions are also incrementing the refcount on their own, * so it's safe to remove the entry even if it's in use. */ @@ -176,32 +174,37 @@ clusterip_netdev_event(struct notifier_block *this, unsigned long event, void *ptr) { struct net_device *dev = netdev_notifier_info_to_dev(ptr); + struct net *net = dev_net(dev); + struct clusterip_net *cn = net_generic(net, clusterip_net_id); struct clusterip_config *c; - c = container_of(this, struct clusterip_config, notifier); - switch (event) { - case NETDEV_REGISTER: - if (!strcmp(dev->name, c->ifname)) { - c->ifindex = dev->ifindex; - dev_mc_add(dev, c->clustermac); - } - break; - case NETDEV_UNREGISTER: - if (dev->ifindex == c->ifindex) { - dev_mc_del(dev, c->clustermac); - c->ifindex = -1; - } - break; - case NETDEV_CHANGENAME: - if (!strcmp(dev->name, c->ifname)) { - c->ifindex = dev->ifindex; - dev_mc_add(dev, c->clustermac); - } else if (dev->ifindex == c->ifindex) { - dev_mc_del(dev, c->clustermac); - c->ifindex = -1; + rcu_read_lock(); + list_for_each_entry_rcu(c, &cn->configs, list) { + switch (event) { + case NETDEV_REGISTER: + if (!strcmp(dev->name, c->ifname)) { + c->ifindex = dev->ifindex; + dev_mc_add(dev, c->clustermac); + } + break; + case NETDEV_UNREGISTER: + if (dev->ifindex == c->ifindex) { + dev_mc_del(dev, c->clustermac); + c->ifindex = -1; + } + break; + case NETDEV_CHANGENAME: + if (!strcmp(dev->name, c->ifname)) { + c->ifindex = dev->ifindex; + dev_mc_add(dev, c->clustermac); + } else if (dev->ifindex == c->ifindex) { + dev_mc_del(dev, c->clustermac); + c->ifindex = -1; + } + break; } - break; } + rcu_read_unlock(); return NOTIFY_DONE; } @@ -256,11 +259,7 @@ clusterip_config_init(struct net *net, const struct ipt_clusterip_tgt_info *i, } #endif - c->notifier.notifier_call = clusterip_netdev_event; - err = register_netdevice_notifier(&c->notifier); - if (!err) - return c; - + return c; #ifdef CONFIG_PROC_FS proc_remove(c->pde); err: @@ -798,9 +797,17 @@ static int clusterip_net_init(struct net *net) if (ret < 0) return ret; + cn->notifier.notifier_call = clusterip_netdev_event; + ret = register_netdevice_notifier(&cn->notifier); + if (ret) { + nf_unregister_net_hook(net, &cip_arp_ops); + return ret; + } + #ifdef CONFIG_PROC_FS cn->procdir = proc_mkdir("ipt_CLUSTERIP", net->proc_net); if (!cn->procdir) { + unregister_netdevice_notifier(&cn->notifier); nf_unregister_net_hook(net, &cip_arp_ops); pr_err("Unable to proc dir entry\n"); return -ENOMEM; @@ -812,10 +819,11 @@ static int clusterip_net_init(struct net *net) static void clusterip_net_exit(struct net *net) { -#ifdef CONFIG_PROC_FS struct clusterip_net *cn = net_generic(net, clusterip_net_id); +#ifdef CONFIG_PROC_FS proc_remove(cn->procdir); #endif + unregister_netdevice_notifier(&cn->notifier); nf_unregister_net_hook(net, &cip_arp_ops); }