netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* net: lockdep warning in ip_mc_msfget (net/ipv4/igmp.c:2400)
@ 2015-11-02 15:22 Sasha Levin
  2015-11-02 21:31 ` Cong Wang
  0 siblings, 1 reply; 4+ messages in thread
From: Sasha Levin @ 2015-11-02 15:22 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, LKML, syzkaller, Dmitry Vyukov

Hi all,

While fuzzing with syzkaller inside a KVM tools guest running the latest -next, I saw
the following warning:

[ 2391.993558] ======================================================
[ 2391.995441] [ INFO: possible circular locking dependency detected ]
[ 2391.995771] 4.3.0-rc6-next-20151022-sasha-00042-g2b253a1-dirty #2618 Not tainted
[ 2391.995771] -------------------------------------------------------
[ 2391.995771] syzkaller_execu/14105 is trying to acquire lock:
[ 2391.995771] (rtnl_mutex){+.+.+.}, at: rtnl_lock (net/core/rtnetlink.c:71)
[ 2391.995771] Mutex: counter: 1 owner: None
[ 2391.995771]
[ 2391.995771] but task is already holding lock:
[ 2391.995771] (sk_lock-AF_INET){+.+.+.}, at: do_ip_getsockopt (net/ipv4/ip_sockglue.c:1274)
[ 2391.995771]
[ 2391.995771] which lock already depends on the new lock.
[ 2391.995771]
[ 2391.995771]
[ 2391.995771] the existing dependency chain (in reverse order) is:
[ 2391.995771] -> #1 (sk_lock-AF_INET){+.+.+.}:
[ 2391.995771] lock_acquire (kernel/locking/lockdep.c:3620)
[ 2391.995771] lock_sock_nested (include/linux/bottom_half.h:31 net/core/sock.c:2411)
[ 2391.995771] do_ip_setsockopt.isra.9 (net/ipv4/ip_sockglue.c:623)
[ 2391.995771] ip_setsockopt (net/ipv4/ip_sockglue.c:1202)
[ 2391.995771]        ffffffffffffff, 0x0)
[ 2391.995771] sock_common_setsockopt (net/core/sock.c:2610)
[ 2391.995771] SyS_setsockopt (net/socket.c:1756 net/socket.c:1736)
[ 2391.995771] entry_SYSCALL_64_fastpath (arch/x86/entry/entry_64.S:188)
[ 2391.995771] -> #0 (rtnl_mutex){+.+.+.}:
[ 2391.995771] __lock_acquire (kernel/locking/lockdep.c:1877 kernel/locking/lockdep.c:1982 kernel/locking/lockdep.c:2168 kernel/locking/lockdep.c:3239)
[ 2391.995771] lock_acquire (kernel/locking/lockdep.c:3620)
[ 2391.995771] mutex_lock_nested (kernel/locking/mutex.c:526 kernel/locking/mutex.c:618)
[ 2391.995771] rtnl_lock (net/core/rtnetlink.c:71)
[ 2391.995771] ip_mc_msfget (net/ipv4/igmp.c:2400)
[ 2391.995771] do_ip_getsockopt (net/ipv4/ip_sockglue.c:1401)
[ 2391.995771] ip_getsockopt (net/ipv4/ip_sockglue.c:1498)
[ 2391.995771] raw_getsockopt (net/ipv4/raw.c:851)
[ 2391.995771] sock_common_getsockopt (net/core/sock.c:2569)
[ 2391.995771] SyS_getsockopt (net/socket.c:1787 net/socket.c:1770)
[ 2391.995771] entry_SYSCALL_64_fastpath (arch/x86/entry/entry_64.S:188)
[ 2391.995771]
[ 2391.995771] other info that might help us debug this:
[ 2391.995771]
[ 2391.995771]  Possible unsafe locking scenario:
[ 2391.995771]
[ 2391.995771]        CPU0                    CPU1
[ 2391.995771]        ----                    ----
[ 2391.995771]   lock(sk_lock-AF_INET);
[ 2391.995771]                                lock(rtnl_mutex);
[ 2391.995771]                                lock(sk_lock-AF_INET);
[ 2391.995771]   lock(rtnl_mutex);
[ 2391.995771]
[ 2391.995771]  *** DEADLOCK ***
[ 2391.995771]
[ 2391.995771] 1 lock held by syzkaller_execu/14105:
[ 2391.995771] #0: (sk_lock-AF_INET){+.+.+.}, at: do_ip_getsockopt (net/ipv4/ip_sockglue.c:1274)
[ 2391.995771]
[ 2391.995771] stack backtrace:
[ 2391.995771] CPU: 1 PID: 14105 Comm: syzkaller_execu Not tainted 4.3.0-rc6-next-20151022-sasha-00042-g2b253a1-dirty #2618
[ 2391.995771]  0000000000000001 00000000c179c8c9 ffff8800a403f550 ffffffffade32a2b
[ 2391.995771]  ffffffffbb7f5a50 ffffffffbb84a4a0 ffffffffbb7f5a50 ffff8800a403f5a0
[ 2391.995771]  ffffffffac43fca8 ffff8800a403f690 00000000a3e18000 ffff8800a3e18000
[ 2391.995771] Call Trace:
[ 2391.995771] dump_stack (lib/dump_stack.c:52)
[ 2391.995771] print_circular_bug (kernel/locking/lockdep.c:1250)
[ 2391.995771] __lock_acquire (kernel/locking/lockdep.c:1877 kernel/locking/lockdep.c:1982 kernel/locking/lockdep.c:2168 kernel/locking/lockdep.c:3239)
[ 2391.995771] lock_acquire (kernel/locking/lockdep.c:3620)
[ 2391.995771] mutex_lock_nested (kernel/locking/mutex.c:526 kernel/locking/mutex.c:618)
[ 2391.995771] rtnl_lock (net/core/rtnetlink.c:71)
[ 2391.995771] ip_mc_msfget (net/ipv4/igmp.c:2400)
[ 2391.995771] do_ip_getsockopt (net/ipv4/ip_sockglue.c:1401)
[ 2391.995771] ip_getsockopt (net/ipv4/ip_sockglue.c:1498)
[ 2391.995771] raw_getsockopt (net/ipv4/raw.c:851)
[ 2391.995771] sock_common_getsockopt (net/core/sock.c:2569)
[ 2391.995771] SyS_getsockopt (net/socket.c:1787 net/socket.c:1770)
[ 2391.995771] entry_SYSCALL_64_fastpath (arch/x86/entry/entry_64.S:188)


Thanks,
Sasha

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: net: lockdep warning in ip_mc_msfget (net/ipv4/igmp.c:2400)
  2015-11-02 15:22 net: lockdep warning in ip_mc_msfget (net/ipv4/igmp.c:2400) Sasha Levin
@ 2015-11-02 21:31 ` Cong Wang
  2015-11-03  0:38   ` Cong Wang
  0 siblings, 1 reply; 4+ messages in thread
From: Cong Wang @ 2015-11-02 21:31 UTC (permalink / raw)
  To: Sasha Levin
  Cc: netdev, David S. Miller, LKML, syzkaller, Dmitry Vyukov, marcelo.leitner

On Mon, Nov 2, 2015 at 7:22 AM, Sasha Levin <sasha.levin@oracle.com> wrote:
> Hi all,
>
> While fuzzing with syzkaller inside a KVM tools guest running the latest -next, I saw
> the following warning:
>
> [ 2391.993558] ======================================================
> [ 2391.995441] [ INFO: possible circular locking dependency detected ]
> [ 2391.995771] 4.3.0-rc6-next-20151022-sasha-00042-g2b253a1-dirty #2618 Not tainted
> [ 2391.995771] -------------------------------------------------------
> [ 2391.995771] syzkaller_execu/14105 is trying to acquire lock:
> [ 2391.995771] (rtnl_mutex){+.+.+.}, at: rtnl_lock (net/core/rtnetlink.c:71)
> [ 2391.995771] Mutex: counter: 1 owner: None
> [ 2391.995771]
> [ 2391.995771] but task is already holding lock:
> [ 2391.995771] (sk_lock-AF_INET){+.+.+.}, at: do_ip_getsockopt (net/ipv4/ip_sockglue.c:1274)
> [ 2391.995771]
> [ 2391.995771] which lock already depends on the new lock.
> [ 2391.995771]
> [ 2391.995771]
> [ 2391.995771] the existing dependency chain (in reverse order) is:
> [ 2391.995771] -> #1 (sk_lock-AF_INET){+.+.+.}:
> [ 2391.995771] lock_acquire (kernel/locking/lockdep.c:3620)
> [ 2391.995771] lock_sock_nested (include/linux/bottom_half.h:31 net/core/sock.c:2411)
> [ 2391.995771] do_ip_setsockopt.isra.9 (net/ipv4/ip_sockglue.c:623)
> [ 2391.995771] ip_setsockopt (net/ipv4/ip_sockglue.c:1202)
> [ 2391.995771]        ffffffffffffff, 0x0)
> [ 2391.995771] sock_common_setsockopt (net/core/sock.c:2610)
> [ 2391.995771] SyS_setsockopt (net/socket.c:1756 net/socket.c:1736)
> [ 2391.995771] entry_SYSCALL_64_fastpath (arch/x86/entry/entry_64.S:188)
> [ 2391.995771] -> #0 (rtnl_mutex){+.+.+.}:
> [ 2391.995771] __lock_acquire (kernel/locking/lockdep.c:1877 kernel/locking/lockdep.c:1982 kernel/locking/lockdep.c:2168 kernel/locking/lockdep.c:3239)
> [ 2391.995771] lock_acquire (kernel/locking/lockdep.c:3620)
> [ 2391.995771] mutex_lock_nested (kernel/locking/mutex.c:526 kernel/locking/mutex.c:618)
> [ 2391.995771] rtnl_lock (net/core/rtnetlink.c:71)
> [ 2391.995771] ip_mc_msfget (net/ipv4/igmp.c:2400)
> [ 2391.995771] do_ip_getsockopt (net/ipv4/ip_sockglue.c:1401)
> [ 2391.995771] ip_getsockopt (net/ipv4/ip_sockglue.c:1498)
> [ 2391.995771] raw_getsockopt (net/ipv4/raw.c:851)
> [ 2391.995771] sock_common_getsockopt (net/core/sock.c:2569)
> [ 2391.995771] SyS_getsockopt (net/socket.c:1787 net/socket.c:1770)
> [ 2391.995771] entry_SYSCALL_64_fastpath (arch/x86/entry/entry_64.S:188)
> [ 2391.995771]
> [ 2391.995771] other info that might help us debug this:
> [ 2391.995771]
> [ 2391.995771]  Possible unsafe locking scenario:
> [ 2391.995771]
> [ 2391.995771]        CPU0                    CPU1
> [ 2391.995771]        ----                    ----
> [ 2391.995771]   lock(sk_lock-AF_INET);
> [ 2391.995771]                                lock(rtnl_mutex);
> [ 2391.995771]                                lock(sk_lock-AF_INET);
> [ 2391.995771]   lock(rtnl_mutex);
> [ 2391.995771]
> [ 2391.995771]  *** DEADLOCK ***
> [ 2391.995771]
> [ 2391.995771] 1 lock held by syzkaller_execu/14105:
> [ 2391.995771] #0: (sk_lock-AF_INET){+.+.+.}, at: do_ip_getsockopt (net/ipv4/ip_sockglue.c:1274)
> [ 2391.995771]
> [ 2391.995771] stack backtrace:
> [ 2391.995771] CPU: 1 PID: 14105 Comm: syzkaller_execu Not tainted 4.3.0-rc6-next-20151022-sasha-00042-g2b253a1-dirty #2618
> [ 2391.995771]  0000000000000001 00000000c179c8c9 ffff8800a403f550 ffffffffade32a2b
> [ 2391.995771]  ffffffffbb7f5a50 ffffffffbb84a4a0 ffffffffbb7f5a50 ffff8800a403f5a0
> [ 2391.995771]  ffffffffac43fca8 ffff8800a403f690 00000000a3e18000 ffff8800a3e18000
> [ 2391.995771] Call Trace:
> [ 2391.995771] dump_stack (lib/dump_stack.c:52)
> [ 2391.995771] print_circular_bug (kernel/locking/lockdep.c:1250)
> [ 2391.995771] __lock_acquire (kernel/locking/lockdep.c:1877 kernel/locking/lockdep.c:1982 kernel/locking/lockdep.c:2168 kernel/locking/lockdep.c:3239)
> [ 2391.995771] lock_acquire (kernel/locking/lockdep.c:3620)
> [ 2391.995771] mutex_lock_nested (kernel/locking/mutex.c:526 kernel/locking/mutex.c:618)
> [ 2391.995771] rtnl_lock (net/core/rtnetlink.c:71)
> [ 2391.995771] ip_mc_msfget (net/ipv4/igmp.c:2400)
> [ 2391.995771] do_ip_getsockopt (net/ipv4/ip_sockglue.c:1401)
> [ 2391.995771] ip_getsockopt (net/ipv4/ip_sockglue.c:1498)
> [ 2391.995771] raw_getsockopt (net/ipv4/raw.c:851)
> [ 2391.995771] sock_common_getsockopt (net/core/sock.c:2569)
> [ 2391.995771] SyS_getsockopt (net/socket.c:1787 net/socket.c:1770)
> [ 2391.995771] entry_SYSCALL_64_fastpath (arch/x86/entry/entry_64.S:188)

Good catch!

This is probably introduced by:

commit baf606d9c9b12517e47e0d1370e8aa9f7323f210
Author: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date:   Wed Mar 18 14:50:42 2015 -0300

    ipv4,ipv6: grab rtnl before locking the socket

I am thinking what is the right way to fix it...

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: net: lockdep warning in ip_mc_msfget (net/ipv4/igmp.c:2400)
  2015-11-02 21:31 ` Cong Wang
@ 2015-11-03  0:38   ` Cong Wang
  2015-11-03 11:21     ` Marcelo Ricardo Leitner
  0 siblings, 1 reply; 4+ messages in thread
From: Cong Wang @ 2015-11-03  0:38 UTC (permalink / raw)
  To: Sasha Levin
  Cc: netdev, David S. Miller, LKML, syzkaller, Dmitry Vyukov, marcelo.leitner

[-- Attachment #1: Type: text/plain, Size: 430 bytes --]

On Mon, Nov 2, 2015 at 1:31 PM, Cong Wang <cwang@twopensource.com> wrote:
>
> Good catch!
>
> This is probably introduced by:
>
> commit baf606d9c9b12517e47e0d1370e8aa9f7323f210
> Author: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Date:   Wed Mar 18 14:50:42 2015 -0300
>
>     ipv4,ipv6: grab rtnl before locking the socket
>
> I am thinking what is the right way to fix it...

Please try the attached patch.

Thanks!

[-- Attachment #2: tmp.patch --]
[-- Type: text/x-patch, Size: 4066 bytes --]

diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index d38b8b6..a2429b7 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -2392,11 +2392,11 @@ int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf,
 	struct ip_sf_socklist *psl;
 	struct net *net = sock_net(sk);
 
+	ASSERT_RTNL();
+
 	if (!ipv4_is_multicast(addr))
 		return -EINVAL;
 
-	rtnl_lock();
-
 	imr.imr_multiaddr.s_addr = msf->imsf_multiaddr;
 	imr.imr_address.s_addr = msf->imsf_interface;
 	imr.imr_ifindex = 0;
@@ -2417,7 +2417,6 @@ int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf,
 		goto done;
 	msf->imsf_fmode = pmc->sfmode;
 	psl = rtnl_dereference(pmc->sflist);
-	rtnl_unlock();
 	if (!psl) {
 		len = 0;
 		count = 0;
@@ -2436,7 +2435,6 @@ int ip_mc_msfget(struct sock *sk, struct ip_msfilter *msf,
 		return -EFAULT;
 	return 0;
 done:
-	rtnl_unlock();
 	return err;
 }
 
@@ -2450,6 +2448,8 @@ int ip_mc_gsfget(struct sock *sk, struct group_filter *gsf,
 	struct inet_sock *inet = inet_sk(sk);
 	struct ip_sf_socklist *psl;
 
+	ASSERT_RTNL();
+
 	psin = (struct sockaddr_in *)&gsf->gf_group;
 	if (psin->sin_family != AF_INET)
 		return -EINVAL;
@@ -2457,8 +2457,6 @@ int ip_mc_gsfget(struct sock *sk, struct group_filter *gsf,
 	if (!ipv4_is_multicast(addr))
 		return -EINVAL;
 
-	rtnl_lock();
-
 	err = -EADDRNOTAVAIL;
 
 	for_each_pmc_rtnl(inet, pmc) {
@@ -2470,7 +2468,6 @@ int ip_mc_gsfget(struct sock *sk, struct group_filter *gsf,
 		goto done;
 	gsf->gf_fmode = pmc->sfmode;
 	psl = rtnl_dereference(pmc->sflist);
-	rtnl_unlock();
 	count = psl ? psl->sl_count : 0;
 	copycount = count < gsf->gf_numsrc ? count : gsf->gf_numsrc;
 	gsf->gf_numsrc = count;
@@ -2490,7 +2487,6 @@ int ip_mc_gsfget(struct sock *sk, struct group_filter *gsf,
 	}
 	return 0;
 done:
-	rtnl_unlock();
 	return err;
 }
 
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index c3c359a..2b12ed2 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -1251,11 +1251,22 @@ EXPORT_SYMBOL(compat_ip_setsockopt);
  *	the _received_ ones. The set sets the _sent_ ones.
  */
 
+static bool getsockopt_needs_rtnl(int optname)
+{
+	switch (optname) {
+	case IP_MSFILTER:
+	case MCAST_MSFILTER:
+		return true;
+	}
+	return false;
+}
+
 static int do_ip_getsockopt(struct sock *sk, int level, int optname,
 			    char __user *optval, int __user *optlen, unsigned int flags)
 {
 	struct inet_sock *inet = inet_sk(sk);
-	int val;
+	bool needs_rtnl = getsockopt_needs_rtnl(optname);
+	int val, err;
 	int len;
 
 	if (level != SOL_IP)
@@ -1269,6 +1280,8 @@ static int do_ip_getsockopt(struct sock *sk, int level, int optname,
 	if (len < 0)
 		return -EINVAL;
 
+	if (needs_rtnl)
+		rtnl_lock();
 	lock_sock(sk);
 
 	switch (optname) {
@@ -1389,17 +1402,16 @@ static int do_ip_getsockopt(struct sock *sk, int level, int optname,
 		int err;
 
 		if (len < IP_MSFILTER_SIZE(0)) {
-			release_sock(sk);
-			return -EINVAL;
+			err = -EINVAL;
+			goto out;
 		}
 		if (copy_from_user(&msf, optval, IP_MSFILTER_SIZE(0))) {
-			release_sock(sk);
-			return -EFAULT;
+			err = -EFAULT;
+			goto out;
 		}
 		err = ip_mc_msfget(sk, &msf,
 				   (struct ip_msfilter __user *)optval, optlen);
-		release_sock(sk);
-		return err;
+		goto out;
 	}
 	case MCAST_MSFILTER:
 	{
@@ -1407,18 +1419,17 @@ static int do_ip_getsockopt(struct sock *sk, int level, int optname,
 		int err;
 
 		if (len < GROUP_FILTER_SIZE(0)) {
-			release_sock(sk);
-			return -EINVAL;
+			err = -EINVAL;
+			goto out;
 		}
 		if (copy_from_user(&gsf, optval, GROUP_FILTER_SIZE(0))) {
-			release_sock(sk);
-			return -EFAULT;
+			err = -EFAULT;
+			goto out;
 		}
 		err = ip_mc_gsfget(sk, &gsf,
 				   (struct group_filter __user *)optval,
 				   optlen);
-		release_sock(sk);
-		return err;
+		goto out;
 	}
 	case IP_MULTICAST_ALL:
 		val = inet->mc_all;
@@ -1485,6 +1496,12 @@ static int do_ip_getsockopt(struct sock *sk, int level, int optname,
 			return -EFAULT;
 	}
 	return 0;
+
+out:
+	release_sock(sk);
+	if (needs_rtnl)
+		rtnl_unlock();
+	return err;
 }
 
 int ip_getsockopt(struct sock *sk, int level,

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: net: lockdep warning in ip_mc_msfget (net/ipv4/igmp.c:2400)
  2015-11-03  0:38   ` Cong Wang
@ 2015-11-03 11:21     ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 4+ messages in thread
From: Marcelo Ricardo Leitner @ 2015-11-03 11:21 UTC (permalink / raw)
  To: Cong Wang, Sasha Levin
  Cc: netdev, David S. Miller, LKML, syzkaller, Dmitry Vyukov

Em 02-11-2015 22:38, Cong Wang escreveu:
> On Mon, Nov 2, 2015 at 1:31 PM, Cong Wang <cwang@twopensource.com> wrote:
>>
>> Good catch!
>>
>> This is probably introduced by:
>>
>> commit baf606d9c9b12517e47e0d1370e8aa9f7323f210
>> Author: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
>> Date:   Wed Mar 18 14:50:42 2015 -0300
>>
>>      ipv4,ipv6: grab rtnl before locking the socket

Yes, that commit introduced this inverse order situation by not fixing 
the get path of it.

>> I am thinking what is the right way to fix it...
>
> Please try the attached patch.
>
> Thanks!

Patch LGTM, same design and covered all rtnl_lock()s I could find on get 
path now. Thanks!

   Marcelo

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-11-03 11:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-02 15:22 net: lockdep warning in ip_mc_msfget (net/ipv4/igmp.c:2400) Sasha Levin
2015-11-02 21:31 ` Cong Wang
2015-11-03  0:38   ` Cong Wang
2015-11-03 11:21     ` Marcelo Ricardo Leitner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).