{"id":809327,"url":"http://patchwork.ozlabs.org/api/patches/809327/?format=json","web_url":"http://patchwork.ozlabs.org/project/netfilter-devel/patch/20170903161119.15031-1-ap420073@gmail.com/","project":{"id":26,"url":"http://patchwork.ozlabs.org/api/projects/26/?format=json","name":"Netfilter Development","link_name":"netfilter-devel","list_id":"netfilter-devel.vger.kernel.org","list_email":"netfilter-devel@vger.kernel.org","web_url":null,"scm_url":null,"webscm_url":null,"list_archive_url":"","list_archive_url_format":"","commit_url_format":""},"msgid":"<20170903161119.15031-1-ap420073@gmail.com>","list_archive_url":null,"date":"2017-09-03T16:11:19","name":"[V2] netfilter: xt_TEE: Fix potential deadlock when TEE target is inserted","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"e223920ef7130b8102d8f5acb8565164961f056d","submitter":{"id":68997,"url":"http://patchwork.ozlabs.org/api/people/68997/?format=json","name":"Taehee Yoo","email":"ap420073@gmail.com"},"delegate":{"id":6139,"url":"http://patchwork.ozlabs.org/api/users/6139/?format=json","username":"pablo","first_name":"Pablo","last_name":"Neira","email":"pablo@netfilter.org"},"mbox":"http://patchwork.ozlabs.org/project/netfilter-devel/patch/20170903161119.15031-1-ap420073@gmail.com/mbox/","series":[{"id":1253,"url":"http://patchwork.ozlabs.org/api/series/1253/?format=json","web_url":"http://patchwork.ozlabs.org/project/netfilter-devel/list/?series=1253","date":"2017-09-03T16:11:19","name":"[V2] netfilter: xt_TEE: Fix potential deadlock when TEE target is inserted","version":2,"mbox":"http://patchwork.ozlabs.org/series/1253/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/809327/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/809327/checks/","tags":{},"related":[],"headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netfilter-devel-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"JSU45H3A\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xldJb5X7Fz9t33\n\tfor ;\n\tMon, 4 Sep 2017 02:11:27 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753141AbdICQL0 (ORCPT );\n\tSun, 3 Sep 2017 12:11:26 -0400","from mail-pg0-f65.google.com ([74.125.83.65]:33098 \"EHLO\n\tmail-pg0-f65.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1753138AbdICQL0 (ORCPT\n\t);\n\tSun, 3 Sep 2017 12:11:26 -0400","by mail-pg0-f65.google.com with SMTP id m15so3083004pgc.0\n\tfor ;\n\tSun, 03 Sep 2017 09:11:25 -0700 (PDT)","from ap-To-be-filled-by-O-E-M.8.8.8.8 ([222.98.178.163])\n\tby smtp.gmail.com with ESMTPSA id\n\th28sm7265367pfd.142.2017.09.03.09.11.23\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tSun, 03 Sep 2017 09:11:24 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=gmail.com; s=20161025;\n\th=from:to:cc:subject:date:message-id;\n\tbh=77ja6cx9WwUYY2rvHfJ32PKF23KWhzX5jgJr476HWcw=;\n\tb=JSU45H3Axx6wYfumpFYDl969trlC6Ax2ab+3LOa50p78hY3hYfateSRmwXHu+CHW/U\n\tZAgmGmMtHid4PQ2Su2rSf1oQQzdJ0VRk5oansTEfQHjODKgDPe5jKwoZlJ5FKcds4FMI\n\t80BfQrmqe3Mod9naEE0LYxA974cwO1E9K8BxQ8TtkJrcXkX2x4Hjz+hXphkhm+Jt7+R3\n\tLznxvj6oyPCOHUrTEco8mwcdTy9XN0l2yYJIIFm75P84WzBxxo81ONJ89TS59DQ0XlSR\n\t5/qrUazBMpAyOIGT/w1mlRgOZ+cudOVPNUQl7eVhm2AkHgNOseq7NQoS27xaVMFQVDmU\n\t+lEw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id;\n\tbh=77ja6cx9WwUYY2rvHfJ32PKF23KWhzX5jgJr476HWcw=;\n\tb=e1actjtD4rfrLfzqWO9J1rhqd5gM7nlfhZdAfNw1+now0/nzexdb3AJxWkKIzvxQJg\n\tOmUcn4EdRvtc/jESvwj9R1/OcJJzLAPRd1c9msw/tksvdjNsZS9FzxuxgWpEMMegRca5\n\tLUlUi4VZwwszArsK6skGUJBCJyE3nA9bV0Tk0k7nAfGj1pCK62U9xVWAOYL8kheX2h+G\n\t0IhU/aG75w11eEVKoJllffgQJeKK7QnPkQ/eNdVcOr4NEVkG4uti87HvUbv1CJevawkz\n\tnUFYP4HlOwrLHSpu9qP4eYgrH6uI08QxVIHGldSptvHuAYm9a+fZVVk+BHGQKRlnK9Wm\n\tJTWw==","X-Gm-Message-State":"AHPjjUhcmW5PRCl2PeXhO4xR+U1ZmDG1Aa0vgLmK41r7nuqyt4WApWhM\n\tOeOKb25nsa2TTg==","X-Google-Smtp-Source":"ADKCNb5EB6eJ+XZiv7v/RmC9viSm7d+H1U7gCIvwO2nI1VbYZr4XwNFmDXtNAXO8LohOPPb4Uc2ojg==","X-Received":"by 10.84.193.131 with SMTP id f3mr9642398pld.88.1504455085298;\n\tSun, 03 Sep 2017 09:11:25 -0700 (PDT)","From":"Taehee Yoo ","To":"pablo@netfilter.org, jengelh@inai.de, netfilter-devel@vger.kernel.org","Cc":"ap420073@gmail.com","Subject":"[PATCH V2] netfilter: xt_TEE: Fix potential deadlock when TEE\n\ttarget is inserted","Date":"Mon, 4 Sep 2017 01:11:19 +0900","Message-Id":"<20170903161119.15031-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"},"content":"When xt_TEE target is inserted, lockdep warns about possible\nDEADLOCK situation. to avoid deadlock situation\nthe register_netdevice_notifier() should be called by only init routine.\n\nreproduce command is :\n # iptables -I INPUT -j TEE --oif enp3s0 --gateway 192.168.0.1\n\nwarning message is :\n\n[ 115.182917] WARNING: possible circular locking dependency detected\n[ 115.189846] 4.13.0-rc1+ #68 Not tainted\n[ 115.194141] ------------------------------------------------------\n[ 115.201065] iptables/1283 is trying to acquire lock:\n[ 115.206627] (rtnl_mutex){+.+.+.}, at: [] rtnl_lock+0x17/0x20\n[ 115.214842]\n[ 115.214842] but task is already holding lock:\n[ 115.221378] (sk_lock-AF_INET){+.+.+.}, at: [] ip_setsockopt+0x6d/0xb0\n[ 115.230462]\n[ 115.230462] which lock already depends on the new lock.\n[ 115.230462]\n[ 115.239627]\n[ 115.239627] the existing dependency chain (in reverse order) is:\n[ 115.248012]\n[ 115.248012] -> #1 (sk_lock-AF_INET){+.+.+.}:\n[ 115.254472] lock_acquire+0x190/0x370\n[ 115.259165] lock_sock_nested+0xb8/0x100\n[ 115.264148] do_ip_setsockopt.isra.16+0x140/0x24f0\n[ 115.270125] ip_setsockopt+0x34/0xb0\n[ 115.274742] udp_setsockopt+0x1b/0x30\n[ 115.279455] sock_common_setsockopt+0x78/0xf0\n[ 115.284937] SyS_setsockopt+0x11c/0x220\n[ 115.289835] do_syscall_64+0x187/0x410\n[ 115.294638] return_from_SYSCALL_64+0x0/0x7a\n[ 115.300025]\n[ 115.300025] -> #0 (rtnl_mutex){+.+.+.}:\n[ 115.306030] __lock_acquire+0x4114/0x47c0\n[ 115.311132] lock_acquire+0x190/0x370\n[ 115.315844] __mutex_lock+0xef/0x1460\n[ 115.320555] mutex_lock_nested+0x1b/0x20\n[ 115.325558] rtnl_lock+0x17/0x20\n[ 115.329785] register_netdevice_notifier+0x6f/0x4f0\n[ 115.335851] tee_tg_check+0x19b/0x260\n[ 115.340562] xt_check_target+0x1f5/0x6c0\n[ 115.345569] find_check_entry.isra.7+0x62f/0x960\n[ 115.351353] translate_table+0xcf2/0x1830\n[ 115.356454] do_ipt_set_ctl+0x1ff/0x3a0\n[ 115.361362] nf_setsockopt+0x61/0xc0\n[ 115.365977] ip_setsockopt+0x82/0xb0\n[ 115.370592] raw_setsockopt+0x73/0xa0\n[ 115.375304] sock_common_setsockopt+0x78/0xf0\n[ 115.380793] SyS_setsockopt+0x11c/0x220\n[ 115.385701] entry_SYSCALL_64_fastpath+0x1c/0xb1\n[ 115.391478]\n[ 115.391478] other info that might help us debug this:\n[ 115.391478]\n[ 115.400511] Possible unsafe locking scenario:\n[ 115.400511]\n[ 115.407176] CPU0 CPU1\n[ 115.412270] ---- ----\n[ 115.417364] lock(sk_lock-AF_INET);\n[ 115.421394] lock(rtnl_mutex);\n[ 115.427760] lock(sk_lock-AF_INET);\n[ 115.434723] lock(rtnl_mutex);\n[ 115.438267]\n[ 115.438267] *** DEADLOCK ***\n\n[ ... ]\n\nSigned-off-by: Taehee Yoo \n---\n\nV2:\n - Do not modify the xt_TEE.h\n\nV1:\n - Initial version\n\n\n net/netfilter/xt_TEE.c | 89 +++++++++++++++++++++++++++++++-------------------\n 1 file changed, 56 insertions(+), 33 deletions(-)","diff":"diff --git a/net/netfilter/xt_TEE.c b/net/netfilter/xt_TEE.c\nindex 86b0580..2aebbc0 100644\n--- a/net/netfilter/xt_TEE.c\n+++ b/net/netfilter/xt_TEE.c\n@@ -12,19 +12,20 @@\n */\n #include \n #include \n-#include \n #include \n-#include \n #include \n #include \n #include \n \n struct xt_tee_priv {\n-\tstruct notifier_block\tnotifier;\n \tstruct xt_tee_tginfo\t*tginfo;\n+\tstruct net\t\t*net;\n+\tstruct list_head\tlist;\n \tint\t\t\toif;\n };\n \n+static LIST_HEAD(tee_tg_list);\n+static DEFINE_MUTEX(list_mutex);\n static const union nf_inet_addr tee_zero_address;\n \n static unsigned int\n@@ -55,59 +56,68 @@ static int tee_netdev_event(struct notifier_block *this, unsigned long event,\n \t\t\t void *ptr)\n {\n \tstruct net_device *dev = netdev_notifier_info_to_dev(ptr);\n+\tstruct net *net = dev_net(dev);\n \tstruct xt_tee_priv *priv;\n \n-\tpriv = container_of(this, struct xt_tee_priv, notifier);\n-\tswitch (event) {\n-\tcase NETDEV_REGISTER:\n-\t\tif (!strcmp(dev->name, priv->tginfo->oif))\n-\t\t\tpriv->oif = dev->ifindex;\n-\t\tbreak;\n-\tcase NETDEV_UNREGISTER:\n-\t\tif (dev->ifindex == priv->oif)\n-\t\t\tpriv->oif = -1;\n-\t\tbreak;\n-\tcase NETDEV_CHANGENAME:\n-\t\tif (!strcmp(dev->name, priv->tginfo->oif))\n-\t\t\tpriv->oif = dev->ifindex;\n-\t\telse if (dev->ifindex == priv->oif)\n-\t\t\tpriv->oif = -1;\n-\t\tbreak;\n+\tmutex_lock(&list_mutex);\n+\tlist_for_each_entry(priv, &tee_tg_list, list) {\n+\t\tswitch (event) {\n+\t\tcase NETDEV_REGISTER:\n+\t\t\tif (!strcmp(dev->name, priv->tginfo->oif) &&\n+\t\t\t net_eq(net, priv->net))\n+\t\t\t\tpriv->oif = dev->ifindex;\n+\t\t\tbreak;\n+\t\tcase NETDEV_UNREGISTER:\n+\t\t\tif (dev->ifindex == priv->oif && net_eq(net, priv->net))\n+\t\t\t\tpriv->oif = -1;\n+\t\t\tbreak;\n+\t\tcase NETDEV_CHANGENAME:\n+\t\t\tif (!strcmp(dev->name, priv->tginfo->oif) &&\n+\t\t\t net_eq(net, priv->net))\n+\t\t\t\tpriv->oif = dev->ifindex;\n+\t\t\telse if (dev->ifindex == priv->oif &&\n+\t\t\t\t net_eq(net, priv->net))\n+\t\t\t\tpriv->oif = -1;\n+\t\t\tbreak;\n+\t\t}\n \t}\n+\tmutex_unlock(&list_mutex);\n \n \treturn NOTIFY_DONE;\n }\n \n+static struct notifier_block tee_dev_notifier = {\n+\t.notifier_call\t= tee_netdev_event,\n+};\n+\n static int tee_tg_check(const struct xt_tgchk_param *par)\n {\n \tstruct xt_tee_tginfo *info = par->targinfo;\n \tstruct xt_tee_priv *priv;\n \n \t/* 0.0.0.0 and :: not allowed */\n-\tif (memcmp(&info->gw, &tee_zero_address,\n-\t\t sizeof(tee_zero_address)) == 0)\n+\tif (nf_inet_addr_cmp(&info->gw, &tee_zero_address)) {\n+\t\tpr_info(\"TEE: Invalid gateway address\\n\");\n \t\treturn -EINVAL;\n+\t}\n \n \tif (info->oif[0]) {\n-\t\tint ret;\n-\n-\t\tif (info->oif[sizeof(info->oif)-1] != '\\0')\n+\t\tif (info->oif[sizeof(info->oif) - 1] != '\\0') {\n+\t\t\tpr_info(\"TEE: Invalid oif name\\n\");\n \t\t\treturn -EINVAL;\n+\t\t}\n \n \t\tpriv = kzalloc(sizeof(*priv), GFP_KERNEL);\n \t\tif (priv == NULL)\n \t\t\treturn -ENOMEM;\n \n \t\tpriv->tginfo = info;\n+\t\tpriv->net = par->net;\n \t\tpriv->oif = -1;\n-\t\tpriv->notifier.notifier_call = tee_netdev_event;\n \t\tinfo->priv = priv;\n-\n-\t\tret = register_netdevice_notifier(&priv->notifier);\n-\t\tif (ret) {\n-\t\t\tkfree(priv);\n-\t\t\treturn ret;\n-\t\t}\n+\t\tmutex_lock(&list_mutex);\n+\t\tlist_add(&priv->list, &tee_tg_list);\n+\t\tmutex_unlock(&list_mutex);\n \t} else\n \t\tinfo->priv = NULL;\n \n@@ -120,8 +130,10 @@ static void tee_tg_destroy(const struct xt_tgdtor_param *par)\n \tstruct xt_tee_tginfo *info = par->targinfo;\n \n \tif (info->priv) {\n-\t\tunregister_netdevice_notifier(&info->priv->notifier);\n+\t\tmutex_lock(&list_mutex);\n+\t\tlist_del(&info->priv->list);\n \t\tkfree(info->priv);\n+\t\tmutex_unlock(&list_mutex);\n \t}\n \tstatic_key_slow_dec(&xt_tee_enabled);\n }\n@@ -155,11 +167,22 @@ static struct xt_target tee_tg_reg[] __read_mostly = {\n \n static int __init tee_tg_init(void)\n {\n-\treturn xt_register_targets(tee_tg_reg, ARRAY_SIZE(tee_tg_reg));\n+\tint ret;\n+\n+\tret = xt_register_targets(tee_tg_reg, ARRAY_SIZE(tee_tg_reg));\n+\tif (ret)\n+\t\treturn ret;\n+\n+\tret = register_netdevice_notifier(&tee_dev_notifier);\n+\tif (ret)\n+\t\txt_unregister_targets(tee_tg_reg, ARRAY_SIZE(tee_tg_reg));\n+\n+\treturn ret;\n }\n \n static void __exit tee_tg_exit(void)\n {\n+\tunregister_netdevice_notifier(&tee_dev_notifier);\n \txt_unregister_targets(tee_tg_reg, ARRAY_SIZE(tee_tg_reg));\n }\n \n","prefixes":["V2"]}