From patchwork Wed Apr 12 19:32:13 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cong Wang X-Patchwork-Id: 750145 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3w3Db74TTTz9s7x for ; Thu, 13 Apr 2017 05:32:35 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="EgEydSrq"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754445AbdDLTcb (ORCPT ); Wed, 12 Apr 2017 15:32:31 -0400 Received: from mail-pf0-f194.google.com ([209.85.192.194]:33285 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752304AbdDLTc3 (ORCPT ); Wed, 12 Apr 2017 15:32:29 -0400 Received: by mail-pf0-f194.google.com with SMTP id c198so6849855pfc.0 for ; Wed, 12 Apr 2017 12:32:29 -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=24EVEj3pz6JLaQzxGI0G89sGMwphe6YX/v6tkBFV19U=; b=EgEydSrqU7i9S9wa1/YW5Kzu8I/QM2vFBvoXjrlCiLL/9gbO+NdlPeugTqT8Qi+Wqu sX53++ldXfEZjU8MibY0ybCUbPQJHqpxYvSgGcq7T5DGl7mOQiD/5fK6ocRkukz6MaLf BygomoVEv8XVQWwBA4nxzyo3EIEp516PA/0jMkjRqcF0cnEO0HpexLAZp859Du/YSHQo CZ1kaZTjc+jF0C2b92tuNf7+FRzBU/nDrjInsmhI+Tbwp6SYmK7QQZNNi575Rq8G/hI2 Q+VHYgv1vVkmiIjZ+hDerSnDWyQfzLDpTVxOc3HY0Ibb57fdna0opoAHxfoNAswEe4Kb 1b9A== 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=24EVEj3pz6JLaQzxGI0G89sGMwphe6YX/v6tkBFV19U=; b=V5PPMWE+by4F85D6ulgazN5fKt9RTgm9vWhjo0hzloccmawLhOvqJjQmJPX95RdaVF NGQGJE2NnSdYMN4PS+mAUyG/x8UJBeK6wjlJUAlkwkok9r5Kui5NHiFOfnpTgUdkTrnI 9YNy0z32HbiDAaah7gl6Ay7Y84ZZLpMT94h2biPzx9X1HUvTMD4/UW5NJEOQu5D0RJiF zLEDqgJsjT84zGsjqFp6BJ54vLH3o2uz01mCT9hJqa2ibwp5vwTA7vBvSZSGG0mVC0ie QDG2eucpEjisQq4A/1+SVSbe5ietUfMk7LkWXVezsW7OflC1evH2V7REtgT2VpW2IsMy +uqA== X-Gm-Message-State: AN3rC/7xeApYVclQJQg4kP0IDg4ho5Ogg32UaIvJv9XPGGSJdEHzWR62x7iai31ZZnR5zg== X-Received: by 10.84.131.100 with SMTP id 91mr298929pld.40.1492025549237; Wed, 12 Apr 2017 12:32:29 -0700 (PDT) Received: from tw-172-25-29-91.office.twttr.net ([8.25.197.27]) by smtp.gmail.com with ESMTPSA id i3sm38065792pfg.117.2017.04.12.12.32.27 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 12 Apr 2017 12:32:28 -0700 (PDT) From: Cong Wang To: netdev@vger.kernel.org Cc: dvyukov@google.com, andreyknvl@google.com, Cong Wang Subject: [Patch net] ipv4: fix a deadlock in ip_ra_control Date: Wed, 12 Apr 2017 12:32:13 -0700 Message-Id: <1492025533-23084-1-git-send-email-xiyou.wangcong@gmail.com> X-Mailer: git-send-email 2.5.5 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Similar to commit 87e9f0315952 ("ipv4: fix a potential deadlock in mcast getsockopt() path"), there is a deadlock scenario for IP_ROUTER_ALERT too: CPU0 CPU1 ---- ---- lock(rtnl_mutex); lock(sk_lock-AF_INET); lock(rtnl_mutex); lock(sk_lock-AF_INET); Fix this by always locking RTNL first on all setsockopt() paths. Note, after this patch ip_ra_lock is no longer needed either. Reported-by: Dmitry Vyukov Tested-by: Andrey Konovalov Signed-off-by: Cong Wang --- net/ipv4/ip_sockglue.c | 1 + net/ipv4/ipmr.c | 11 ++--------- net/ipv4/raw.c | 2 ++ 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c index ebd953b..bda318a 100644 --- a/net/ipv4/ip_sockglue.c +++ b/net/ipv4/ip_sockglue.c @@ -591,6 +591,7 @@ static bool setsockopt_needs_rtnl(int optname) case MCAST_LEAVE_GROUP: case MCAST_LEAVE_SOURCE_GROUP: case MCAST_UNBLOCK_SOURCE: + case IP_ROUTER_ALERT: return true; } return false; diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c index c0317c9..b036e85 100644 --- a/net/ipv4/ipmr.c +++ b/net/ipv4/ipmr.c @@ -1278,7 +1278,7 @@ static void mrtsock_destruct(struct sock *sk) struct net *net = sock_net(sk); struct mr_table *mrt; - rtnl_lock(); + ASSERT_RTNL(); ipmr_for_each_table(mrt, net) { if (sk == rtnl_dereference(mrt->mroute_sk)) { IPV4_DEVCONF_ALL(net, MC_FORWARDING)--; @@ -1289,7 +1289,6 @@ static void mrtsock_destruct(struct sock *sk) mroute_clean_tables(mrt, false); } } - rtnl_unlock(); } /* Socket options and virtual interface manipulation. The whole @@ -1353,13 +1352,8 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, if (sk != rcu_access_pointer(mrt->mroute_sk)) { ret = -EACCES; } else { - /* We need to unlock here because mrtsock_destruct takes - * care of rtnl itself and we can't change that due to - * the IP_ROUTER_ALERT setsockopt which runs without it. - */ - rtnl_unlock(); ret = ip_ra_control(sk, 0, NULL); - goto out; + goto out_unlock; } break; case MRT_ADD_VIF: @@ -1470,7 +1464,6 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, } out_unlock: rtnl_unlock(); -out: return ret; } diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c index 8119e1f..9d94397 100644 --- a/net/ipv4/raw.c +++ b/net/ipv4/raw.c @@ -682,7 +682,9 @@ static void raw_close(struct sock *sk, long timeout) /* * Raw sockets may have direct kernel references. Kill them. */ + rtnl_lock(); ip_ra_control(sk, 0, NULL); + rtnl_unlock(); sk_common_release(sk); }