On Fri, Mar 3, 2017 at 10:43 AM, Dmitry Vyukov <dvyu...@google.com> wrote:
> On Thu, Mar 2, 2017 at 10:40 AM, Dmitry Vyukov <dvyu...@google.com> wrote:
>> On Wed, Mar 1, 2017 at 6:18 PM, Cong Wang <xiyou.wangc...@gmail.com> wrote:
>>> On Wed, Mar 1, 2017 at 2:44 AM, Dmitry Vyukov <dvyu...@google.com> wrote:
>>>> Hello,
>>>>
>>>> I've got the following deadlock report while running syzkaller fuzzer
>>>> on linux-next/51788aebe7cae79cb334ad50641347465fc188fd:
>>>>
>>>> ======================================================
>>>> [ INFO: possible circular locking dependency detected ]
>>>> 4.10.0-next-20170301+ #1 Not tainted
>>>> -------------------------------------------------------
>>>> syz-executor1/3394 is trying to acquire lock:
>>>> (sk_lock-AF_INET){+.+.+.}, at: [<ffffffff838864cc>] lock_sock
>>>> include/net/sock.h:1460 [inline]
>>>> (sk_lock-AF_INET){+.+.+.}, at: [<ffffffff838864cc>]
>>>> do_ip_setsockopt.isra.12+0x21c/0x3540 net/ipv4/ip_sockglue.c:652
>>>>
>>>> but task is already holding lock:
>>>> (rtnl_mutex){+.+.+.}, at: [<ffffffff836fbd97>] rtnl_lock+0x17/0x20
>>>> net/core/rtnetlink.c:70
>>>>
>>>> which lock already depends on the new lock.
>>>>
>>>>
>>>> the existing dependency chain (in reverse order) is:
>>>>
>>>> -> #1 (rtnl_mutex){+.+.+.}:
>>>> validate_chain kernel/locking/lockdep.c:2265 [inline]
>>>> __lock_acquire+0x2149/0x3430 kernel/locking/lockdep.c:3338
>>>> lock_acquire+0x2a1/0x630 kernel/locking/lockdep.c:3753
>>>> __mutex_lock_common kernel/locking/mutex.c:754 [inline]
>>>> __mutex_lock+0x172/0x1730 kernel/locking/mutex.c:891
>>>> mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:906
>>>> rtnl_lock+0x17/0x20 net/core/rtnetlink.c:70
>>>> mrtsock_destruct+0x86/0x2c0 net/ipv4/ipmr.c:1281
>>>> ip_ra_control+0x459/0x600 net/ipv4/ip_sockglue.c:372
>>>> do_ip_setsockopt.isra.12+0x1064/0x3540 net/ipv4/ip_sockglue.c:1161
>>>> ip_setsockopt+0x3a/0xb0 net/ipv4/ip_sockglue.c:1264
>>>> raw_setsockopt+0xb7/0xd0 net/ipv4/raw.c:839
>>>> sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2725
>>>> SYSC_setsockopt net/socket.c:1786 [inline]
>>>> SyS_setsockopt+0x25c/0x390 net/socket.c:1765
>>>> entry_SYSCALL_64_fastpath+0x1f/0xc2
>>>>
>>>> -> #0 (sk_lock-AF_INET){+.+.+.}:
>>>> check_prev_add kernel/locking/lockdep.c:1828 [inline]
>>>> check_prevs_add+0xa8f/0x19f0 kernel/locking/lockdep.c:1938
>>>> validate_chain kernel/locking/lockdep.c:2265 [inline]
>>>> __lock_acquire+0x2149/0x3430 kernel/locking/lockdep.c:3338
>>>> lock_acquire+0x2a1/0x630 kernel/locking/lockdep.c:3753
>>>> lock_sock_nested+0xcb/0x120 net/core/sock.c:2530
>>>> lock_sock include/net/sock.h:1460 [inline]
>>>> do_ip_setsockopt.isra.12+0x21c/0x3540 net/ipv4/ip_sockglue.c:652
>>>> ip_setsockopt+0x3a/0xb0 net/ipv4/ip_sockglue.c:1264
>>>> tcp_setsockopt+0x82/0xd0 net/ipv4/tcp.c:2721
>>>> sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2725
>>>> SYSC_setsockopt net/socket.c:1786 [inline]
>>>> SyS_setsockopt+0x25c/0x390 net/socket.c:1765
>>>> entry_SYSCALL_64_fastpath+0x1f/0xc2
>>>>
>>>
>>> Please try the attached patch (compile only).
>>
>>
>> Pushed the patch to the bots.
>> Thanks
>
>
> This patch triggers:
Ah, update the patch to fix this.
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);
}
