netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Patch net] ipv6: fix rtnl lock assertion failure in ipv6_sock_ac_join()
@ 2014-09-02 18:07 Cong Wang
  2014-09-02 21:29 ` Hannes Frederic Sowa
  0 siblings, 1 reply; 3+ messages in thread
From: Cong Wang @ 2014-09-02 18:07 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, David S. Miller, Hannes Frederic Sowa, Sabrina Dubroca

Tommi reported the following RTNL lock assertion failure:

[   77.297196] RTNL: assertion failed at net/ipv6/addrconf.c (1699)
[   77.298080] CPU: 0 PID: 4842 Comm: trinity-main Not tainted 3.17.0-rc2+ #30
[   77.299039] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[   77.299789]  ffff88003d76a618 ffff880026133c50 ffffffff8238ba79
ffff880037c84520
[   77.300829]  ffff880026133c90 ffffffff820bd52b 0000000000000000
ffffffff82d86c40
[   77.301869]  0000000000000000 00000000f76fd1e1 ffff8800382d8000
ffff8800382d8220
[   77.302906] Call Trace:
[   77.303246]  [<ffffffff8238ba79>] dump_stack+0x4d/0x66
[   77.303928]  [<ffffffff820bd52b>] addrconf_join_solict+0x4b/0xb0
[   77.304731]  [<ffffffff820b031b>] ipv6_dev_ac_inc+0x2bb/0x330
[   77.305498]  [<ffffffff820b0060>] ? ac6_seq_start+0x260/0x260
[   77.306257]  [<ffffffff820b05fe>] ipv6_sock_ac_join+0x26e/0x360
[   77.307046]  [<ffffffff820b0429>] ? ipv6_sock_ac_join+0x99/0x360
[   77.307798]  [<ffffffff820cdd60>] do_ipv6_setsockopt.isra.5+0xa70/0xf20

This is due to we don't hold rtnl lock when calling addrconf_join_solict()
in ipv6_sock_ac_join(). So hold rtnl lock instead of RCU lock here,
after all it is not a hot path.

BTW, mcast _might_ have similar problem, but I don't touch it since no one
reports so far.

Reported-by: Tommi Rantala <tt.rantala@gmail.com>
Tested-by: Sabrina Dubroca <sd@queasysnail.net>
Cc: David S. Miller <davem@davemloft.net>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/linux/netdevice.h |  4 ++--
 net/core/dev.c            | 13 +++++++------
 net/ipv6/anycast.c        | 22 +++++++++++-----------
 3 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 38377392..71838bb 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2074,8 +2074,8 @@ void __dev_remove_pack(struct packet_type *pt);
 void dev_add_offload(struct packet_offload *po);
 void dev_remove_offload(struct packet_offload *po);
 
-struct net_device *dev_get_by_flags_rcu(struct net *net, unsigned short flags,
-					unsigned short mask);
+struct net_device *__dev_get_by_flags(struct net *net, unsigned short flags,
+				      unsigned short mask);
 struct net_device *dev_get_by_name(struct net *net, const char *name);
 struct net_device *dev_get_by_name_rcu(struct net *net, const char *name);
 struct net_device *__dev_get_by_name(struct net *net, const char *name);
diff --git a/net/core/dev.c b/net/core/dev.c
index ab9a165..343847a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -897,23 +897,24 @@ struct net_device *dev_getfirstbyhwtype(struct net *net, unsigned short type)
 EXPORT_SYMBOL(dev_getfirstbyhwtype);
 
 /**
- *	dev_get_by_flags_rcu - find any device with given flags
+ *	__dev_get_by_flags - find any device with given flags
  *	@net: the applicable net namespace
  *	@if_flags: IFF_* values
  *	@mask: bitmask of bits in if_flags to check
  *
  *	Search for any interface with the given flags. Returns NULL if a device
  *	is not found or a pointer to the device. Must be called inside
- *	rcu_read_lock(), and result refcount is unchanged.
+ *	rtnl_lock(), and result refcount is unchanged.
  */
 
-struct net_device *dev_get_by_flags_rcu(struct net *net, unsigned short if_flags,
-				    unsigned short mask)
+struct net_device *__dev_get_by_flags(struct net *net, unsigned short if_flags,
+				      unsigned short mask)
 {
 	struct net_device *dev, *ret;
 
+	ASSERT_RTNL();
 	ret = NULL;
-	for_each_netdev_rcu(net, dev) {
+	for_each_netdev(net, dev) {
 		if (((dev->flags ^ if_flags) & mask) == 0) {
 			ret = dev;
 			break;
@@ -921,7 +922,7 @@ struct net_device *dev_get_by_flags_rcu(struct net *net, unsigned short if_flags
 	}
 	return ret;
 }
-EXPORT_SYMBOL(dev_get_by_flags_rcu);
+EXPORT_SYMBOL(__dev_get_by_flags);
 
 /**
  *	dev_valid_name - check if name is okay for network device
diff --git a/net/ipv6/anycast.c b/net/ipv6/anycast.c
index 2101832..484ed23 100644
--- a/net/ipv6/anycast.c
+++ b/net/ipv6/anycast.c
@@ -77,7 +77,7 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
 	pac->acl_next = NULL;
 	pac->acl_addr = *addr;
 
-	rcu_read_lock();
+	rtnl_lock();
 	if (ifindex == 0) {
 		struct rt6_info *rt;
 
@@ -90,11 +90,11 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
 			goto error;
 		} else {
 			/* router, no matching interface: just pick one */
-			dev = dev_get_by_flags_rcu(net, IFF_UP,
-						   IFF_UP | IFF_LOOPBACK);
+			dev = __dev_get_by_flags(net, IFF_UP,
+						 IFF_UP | IFF_LOOPBACK);
 		}
 	} else
-		dev = dev_get_by_index_rcu(net, ifindex);
+		dev = __dev_get_by_index(net, ifindex);
 
 	if (dev == NULL) {
 		err = -ENODEV;
@@ -136,7 +136,7 @@ int ipv6_sock_ac_join(struct sock *sk, int ifindex, const struct in6_addr *addr)
 	}
 
 error:
-	rcu_read_unlock();
+	rtnl_unlock();
 	if (pac)
 		sock_kfree_s(sk, pac, sizeof(*pac));
 	return err;
@@ -171,11 +171,11 @@ int ipv6_sock_ac_drop(struct sock *sk, int ifindex, const struct in6_addr *addr)
 
 	spin_unlock_bh(&ipv6_sk_ac_lock);
 
-	rcu_read_lock();
-	dev = dev_get_by_index_rcu(net, pac->acl_ifindex);
+	rtnl_lock();
+	dev = __dev_get_by_index(net, pac->acl_ifindex);
 	if (dev)
 		ipv6_dev_ac_dec(dev, &pac->acl_addr);
-	rcu_read_unlock();
+	rtnl_unlock();
 
 	sock_kfree_s(sk, pac, sizeof(*pac));
 	return 0;
@@ -198,7 +198,7 @@ void ipv6_sock_ac_close(struct sock *sk)
 	spin_unlock_bh(&ipv6_sk_ac_lock);
 
 	prev_index = 0;
-	rcu_read_lock();
+	rtnl_lock();
 	while (pac) {
 		struct ipv6_ac_socklist *next = pac->acl_next;
 
@@ -211,7 +211,7 @@ void ipv6_sock_ac_close(struct sock *sk)
 		sock_kfree_s(sk, pac, sizeof(*pac));
 		pac = next;
 	}
-	rcu_read_unlock();
+	rtnl_unlock();
 }
 
 static void aca_put(struct ifacaddr6 *ac)
@@ -331,7 +331,7 @@ int __ipv6_dev_ac_dec(struct inet6_dev *idev, const struct in6_addr *addr)
 	return 0;
 }
 
-/* called with rcu_read_lock() */
+/* called with rtnl_lock() */
 static int ipv6_dev_ac_dec(struct net_device *dev, const struct in6_addr *addr)
 {
 	struct inet6_dev *idev = __in6_dev_get(dev);
-- 
1.8.3.1

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

* Re: [Patch net] ipv6: fix rtnl lock assertion failure in ipv6_sock_ac_join()
  2014-09-02 18:07 [Patch net] ipv6: fix rtnl lock assertion failure in ipv6_sock_ac_join() Cong Wang
@ 2014-09-02 21:29 ` Hannes Frederic Sowa
  2014-09-02 23:29   ` Cong Wang
  0 siblings, 1 reply; 3+ messages in thread
From: Hannes Frederic Sowa @ 2014-09-02 21:29 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, David S. Miller, Sabrina Dubroca

Hi Cong,

On Di, 2014-09-02 at 11:07 -0700, Cong Wang wrote:
> @@ -198,7 +198,7 @@ void ipv6_sock_ac_close(struct sock *sk)
>  	spin_unlock_bh(&ipv6_sk_ac_lock);
>  
>  	prev_index = 0;
> -	rcu_read_lock();
> +	rtnl_lock();
>  	while (pac) {
>  		struct ipv6_ac_socklist *next = pac->acl_next;
>  
> @@ -211,7 +211,7 @@ void ipv6_sock_ac_close(struct sock *sk)
>  		sock_kfree_s(sk, pac, sizeof(*pac));
>  		pac = next;
>  	}
> -	rcu_read_unlock();
> +	rtnl_unlock();
>  }
>  

Nit:
You forgot to convert a dev_get_by_index_rcu to __dev_get_by_index in
ipv6_sock_ac_close.

Bye,
Hannes

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

* Re: [Patch net] ipv6: fix rtnl lock assertion failure in ipv6_sock_ac_join()
  2014-09-02 21:29 ` Hannes Frederic Sowa
@ 2014-09-02 23:29   ` Cong Wang
  0 siblings, 0 replies; 3+ messages in thread
From: Cong Wang @ 2014-09-02 23:29 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: Cong Wang, netdev, David S. Miller, Sabrina Dubroca

On Tue, Sep 2, 2014 at 2:29 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> Hi Cong,
>
> On Di, 2014-09-02 at 11:07 -0700, Cong Wang wrote:
>> @@ -198,7 +198,7 @@ void ipv6_sock_ac_close(struct sock *sk)
>>       spin_unlock_bh(&ipv6_sk_ac_lock);
>>
>>       prev_index = 0;
>> -     rcu_read_lock();
>> +     rtnl_lock();
>>       while (pac) {
>>               struct ipv6_ac_socklist *next = pac->acl_next;
>>
>> @@ -211,7 +211,7 @@ void ipv6_sock_ac_close(struct sock *sk)
>>               sock_kfree_s(sk, pac, sizeof(*pac));
>>               pac = next;
>>       }
>> -     rcu_read_unlock();
>> +     rtnl_unlock();
>>  }
>>
>
> Nit:
> You forgot to convert a dev_get_by_index_rcu to __dev_get_by_index in
> ipv6_sock_ac_close.
>

Good catch! Will update this patch together with "Fixes:" tag.

Thanks.

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

end of thread, other threads:[~2014-09-02 23:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-02 18:07 [Patch net] ipv6: fix rtnl lock assertion failure in ipv6_sock_ac_join() Cong Wang
2014-09-02 21:29 ` Hannes Frederic Sowa
2014-09-02 23:29   ` Cong Wang

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).