{"id":809301,"url":"http://patchwork.ozlabs.org/api/patches/809301/?format=json","web_url":"http://patchwork.ozlabs.org/project/netfilter-devel/patch/20170903143034.25844-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":"<20170903143034.25844-1-ap420073@gmail.com>","list_archive_url":null,"date":"2017-09-03T14:30:34","name":"netfilter: xt_TEE: Fix potential deadlock when TEE target is inserted","commit_ref":null,"pull_url":null,"state":"changes-requested","archived":false,"hash":"7de6bf78c6fbe1f2b7b75491daebb8963fd6503e","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/20170903143034.25844-1-ap420073@gmail.com/mbox/","series":[{"id":1240,"url":"http://patchwork.ozlabs.org/api/series/1240/?format=json","web_url":"http://patchwork.ozlabs.org/project/netfilter-devel/list/?series=1240","date":"2017-09-03T14:30:34","name":"netfilter: xt_TEE: Fix potential deadlock when TEE target is inserted","version":1,"mbox":"http://patchwork.ozlabs.org/series/1240/mbox/"}],"comments":"http://patchwork.ozlabs.org/api/patches/809301/comments/","check":"pending","checks":"http://patchwork.ozlabs.org/api/patches/809301/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=\"kmx5UNIT\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xlb4R0dt3z9s7v\n\tfor ;\n\tMon, 4 Sep 2017 00:30:46 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752908AbdICOam (ORCPT );\n\tSun, 3 Sep 2017 10:30:42 -0400","from mail-pf0-f193.google.com ([209.85.192.193]:36961 \"EHLO\n\tmail-pf0-f193.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752899AbdICOam (ORCPT\n\t);\n\tSun, 3 Sep 2017 10:30:42 -0400","by mail-pf0-f193.google.com with SMTP id a2so2911609pfj.4\n\tfor ;\n\tSun, 03 Sep 2017 07:30:41 -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\tx12sm7658986pfk.42.2017.09.03.07.30.38\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tSun, 03 Sep 2017 07:30:40 -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=OZ9eOudYsz89R+6Jf6NI/tIWXch2oIS7AphvvKQRRCI=;\n\tb=kmx5UNITlhuzlnYHBVP8AcSjkXUr/d1sXVejI+sgrjG1ICWy3S9MXnCt4XMNJfzwNV\n\tNi97mOD7xZGCSv7MppiJL0Q91DtjDxzIvkH9zVFsBx4XvBuwD8rfea7281eJhIN9UFVb\n\t+1iJGxcM4aKewlRi19N7WhqLKv7MgwhqO9DEgJJxf3PsHjtYDGeLx8Pjfowa/pUZrPms\n\tm4cl6ij4ERaCH3yJs3hD3KyZt9yin/ogMc7TuSNmpZhjH7Q7TKcG0WouToZSycuNIM2V\n\t5slJ9fteBmYnHz6VxJiz8VzraWPd+GkPyMBKeUdXCOVBx1NHxURbdZx37QML9Jq0a6Vg\n\t1eNw==","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=OZ9eOudYsz89R+6Jf6NI/tIWXch2oIS7AphvvKQRRCI=;\n\tb=Gj/ObxhSU2PY6T3kxK7CnwhiLMVFc7suWEsbek3FpWsOo520nfkRfwTS31iesNfozN\n\tVg3qwLD+LW48CVlIvfC/uAG2GkIvVJQU86i5HN2Zq1f+ym4vZ7gnwpxuhtKlApesK0Hy\n\tQMvb/DuMUTLKXYFj1eVFlX6mf4HPkFMvaQJ6WDnyjURiMHnnwED7sxu8Lq/GYHsyFCyF\n\t0ILorviFsj+G+fp6KSnn5Ld68Grq+//IYsxIJ3NMLQO57MEVCvLMQCHxu+35+lFf8At/\n\t0pY4D57DIkodO4MB3XmCOqDXmBlYiF2vy+MTBesiebuI5WwKx5JDd7fpMq54UnRqN2Bk\n\tYcBA==","X-Gm-Message-State":"AHPjjUi9BaZaEPURXponFK9gbgD8OdWL5cT/JtlQsDSHqakaleUmPfVA\n\t/VvjhzolmpyOtw==","X-Google-Smtp-Source":"ADKCNb40HL2TwH1EJ0nKHnKs6xQx9UikCDTd8m/T1vPqg9hqy0sEZHLrcTcDVgItsiLQqbYVLysFMg==","X-Received":"by 10.99.114.76 with SMTP id c12mr9073772pgn.22.1504449041327;\n\tSun, 03 Sep 2017 07:30:41 -0700 (PDT)","From":"Taehee Yoo ","To":"pablo@netfilter.org, netfilter-devel@vger.kernel.org","Cc":"ap420073@gmail.com","Subject":"[PATCH] netfilter: xt_TEE: Fix potential deadlock when TEE target\n\tis inserted","Date":"Sun, 3 Sep 2017 23:30:34 +0900","Message-Id":"<20170903143034.25844-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 include/uapi/linux/netfilter/xt_TEE.h | 3 +-\n net/netfilter/xt_TEE.c | 90 ++++++++++++++++++++++-------------\n 2 files changed, 59 insertions(+), 34 deletions(-)","diff":"diff --git a/include/uapi/linux/netfilter/xt_TEE.h b/include/uapi/linux/netfilter/xt_TEE.h\nindex 0109202..4b7eae4 100644\n--- a/include/uapi/linux/netfilter/xt_TEE.h\n+++ b/include/uapi/linux/netfilter/xt_TEE.h\n@@ -2,10 +2,11 @@\n #define _XT_TEE_TARGET_H\n \n #include \n+#include \n \n struct xt_tee_tginfo {\n \tunion nf_inet_addr gw;\n-\tchar oif[16];\n+\tchar oif[IFNAMSIZ];\n \n \t/* used internally by the kernel */\n \tstruct xt_tee_priv *priv __attribute__((aligned(8)));\ndiff --git a/net/netfilter/xt_TEE.c b/net/netfilter/xt_TEE.c\nindex 86b0580..98fac9f 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,69 @@ 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) &&\n+\t\t\t 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 +131,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 +168,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":[]}