ipv4: fix a deadlock in ip_ra_control

Similar to commit 87e9f03159
("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 <dvyukov@google.com>
Tested-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
WANG Cong 2017-04-12 12:32:13 -07:00 committed by David S. Miller
parent 271a8b428f
commit 1215e51eda
3 changed files with 5 additions and 9 deletions

View File

@ -591,6 +591,7 @@ static bool setsockopt_needs_rtnl(int optname)
case MCAST_LEAVE_GROUP: case MCAST_LEAVE_GROUP:
case MCAST_LEAVE_SOURCE_GROUP: case MCAST_LEAVE_SOURCE_GROUP:
case MCAST_UNBLOCK_SOURCE: case MCAST_UNBLOCK_SOURCE:
case IP_ROUTER_ALERT:
return true; return true;
} }
return false; return false;

View File

@ -1278,7 +1278,7 @@ static void mrtsock_destruct(struct sock *sk)
struct net *net = sock_net(sk); struct net *net = sock_net(sk);
struct mr_table *mrt; struct mr_table *mrt;
rtnl_lock(); ASSERT_RTNL();
ipmr_for_each_table(mrt, net) { ipmr_for_each_table(mrt, net) {
if (sk == rtnl_dereference(mrt->mroute_sk)) { if (sk == rtnl_dereference(mrt->mroute_sk)) {
IPV4_DEVCONF_ALL(net, MC_FORWARDING)--; IPV4_DEVCONF_ALL(net, MC_FORWARDING)--;
@ -1289,7 +1289,6 @@ static void mrtsock_destruct(struct sock *sk)
mroute_clean_tables(mrt, false); mroute_clean_tables(mrt, false);
} }
} }
rtnl_unlock();
} }
/* Socket options and virtual interface manipulation. The whole /* 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)) { if (sk != rcu_access_pointer(mrt->mroute_sk)) {
ret = -EACCES; ret = -EACCES;
} else { } 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); ret = ip_ra_control(sk, 0, NULL);
goto out; goto out_unlock;
} }
break; break;
case MRT_ADD_VIF: case MRT_ADD_VIF:
@ -1470,7 +1464,6 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval,
} }
out_unlock: out_unlock:
rtnl_unlock(); rtnl_unlock();
out:
return ret; return ret;
} }

View File

@ -682,7 +682,9 @@ static void raw_close(struct sock *sk, long timeout)
/* /*
* Raw sockets may have direct kernel references. Kill them. * Raw sockets may have direct kernel references. Kill them.
*/ */
rtnl_lock();
ip_ra_control(sk, 0, NULL); ip_ra_control(sk, 0, NULL);
rtnl_unlock();
sk_common_release(sk); sk_common_release(sk);
} }