NULL pointer dereference in veth_stats_one

NULL pointer dereference in veth_stats_one

[Tom Parkin]

[-- Attachment #1.1: Type: text/plain, Size: 670 bytes --]

Hi list,

I recently tripped over a NULL pointer dereference in the veth driver.
I'm running a 3.8.0_rc1 (updated from net-next git tree this morning)
on an Athlon 64 X2 machine running a 32 bit kernel.  To trigger the
oops I simply created a veth interface as follows:

        ip link add name ve0 type veth peer name ve1

I did a little digging in the git history and I note that veth
statistics changed a little with commit 2681128f0ced8aa4.  I tried
reverting that commit in my tree, which made the oops go away again.

Thanks,
Tom
-- 
Tom Parkin
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development

[-- Attachment #1.2: veth-koops.txt --]
[-- Type: text/plain, Size: 4219 bytes --]

[  266.169346] BUG: unable to handle kernel NULL pointer dereference at 000002c0
[  266.172053] IP: [<f8177388>] veth_stats_one.isra.5+0x38/0xd0 [veth]
[  266.172053] *pde = 00000000 
[  266.172053] Oops: 0000 [#1] SMP 
[  266.172053] Modules linked in: veth bridge stp llc l2tp_ip6 l2tp_ip l2tp_ppp pppox l2tp_eth l2tp_netlink l2tp_core radeon k9
[  266.193196] Pid: 1544, comm: ip Not tainted 3.8.0-rc1-tpdev-23-lockdep+ #29 Gigabyte Technology Co., Ltd. GA-MA69VM-S2/GA-M2
[  266.193196] EIP: 0060:[<f8177388>] EFLAGS: 00010297 CPU: 1
[  266.193196] EIP is at veth_stats_one.isra.5+0x38/0xd0 [veth]
[  266.193196] EAX: 00000000 EBX: f47cd86c ECX: 00000000 EDX: 00000000
[  266.193196] ESI: f47cd874 EDI: 00000000 EBP: f47cd864 ESP: f47cd840
[  266.193196]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[  266.193196] CR0: 8005003b CR2: 000002c0 CR3: 34456000 CR4: 000007d0
[  266.193196] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
[  266.193196] DR6: ffff0ff0 DR7: 00000400
[  266.193196] Process ip (pid: 1544, ti=f47cc000 task=f732bf00 task.ti=f47cc000)
[  266.193196] Stack:
[  266.193196]  f47cd86c 00000003 00000000 00000000 00000000 00000000 f47cd8d4 f345e000
[  266.193196]  f47cd98c f47cd888 f817746d 00000000 00000000 00000000 00000000 c13083d2
[  266.193196]  f47cd8d4 f345e000 f47cd8a0 c14fa6ab f8178040 f345f800 ffffffa6 f345f8bc
[  266.193196] Call Trace:
[  266.193196]  [<f817746d>] veth_get_stats64+0x4d/0x80 [veth]
[  266.193196]  [<c13083d2>] ? __nla_reserve+0x42/0x60
[  266.193196]  [<c14fa6ab>] dev_get_stats+0x5b/0x100
[  266.193196]  [<c15111e9>] rtnl_fill_ifinfo+0x4d9/0xc30
[  266.193196]  [<c109e2e6>] ? mark_held_locks+0x66/0xf0
[  266.193196]  [<c1155572>] ? __kmalloc_track_caller+0xc2/0x1e0
[  266.193196]  [<c14f201e>] ? __alloc_skb+0x5e/0x260
[  266.193196]  [<c14f1f39>] ? __kmalloc_reserve.isra.58+0x29/0x70
[  266.193196]  [<c14f202d>] ? __alloc_skb+0x6d/0x260
[  266.193196]  [<c15128ed>] rtmsg_ifinfo+0x7d/0x100
[  266.193196]  [<c1512a28>] rtnl_configure_link+0x78/0xa0
[  266.193196]  [<f8177633>] veth_newlink+0x143/0x30c [veth]
[  266.193196]  [<c109e2e6>] ? mark_held_locks+0x66/0xf0
[  266.193196]  [<f81774f0>] ? veth_open+0x50/0x50 [veth]
[  266.193196]  [<c1512edc>] rtnl_newlink+0x48c/0x540
[  266.193196]  [<c1512b5f>] ? rtnl_newlink+0x10f/0x540
[  266.193196]  [<c1512a50>] ? rtnl_configure_link+0xa0/0xa0
[  266.193196]  [<c1512693>] rtnetlink_rcv_msg+0x153/0x2a0
[  266.193196]  [<c160cbea>] ? mutex_lock_nested+0x21a/0x2e0
[  266.193196]  [<c150f534>] ? rtnl_lock+0x14/0x20
[  266.193196]  [<c1512540>] ? __rtnl_unlock+0x20/0x20
[  266.193196]  [<c15294be>] netlink_rcv_skb+0x8e/0xb0
[  266.193196]  [<c150f55c>] rtnetlink_rcv+0x1c/0x30
[  266.193196]  [<c1528e7d>] netlink_unicast+0x17d/0x1f0
[  266.193196]  [<c1529114>] netlink_sendmsg+0x224/0x390
[  266.193196]  [<c14e86c1>] sock_sendmsg+0xd1/0xf0
[  266.193196]  [<c1135b89>] ? might_fault+0x89/0x90
[  266.193196]  [<c12fa932>] ? _copy_from_user+0x42/0x60
[  266.193196]  [<c14f5c14>] ? verify_iovec+0x44/0xb0
[  266.193196]  [<c14e95d2>] __sys_sendmsg+0x262/0x270
[  266.193196]  [<c1073e4f>] ? sched_clock_cpu+0xcf/0x150
[  266.193196]  [<c109c02b>] ? trace_hardirqs_off+0xb/0x10
[  266.193196]  [<c1073f35>] ? local_clock+0x65/0x70
[  266.193196]  [<c109c69c>] ? lock_release_holdtime.part.23+0xbc/0xf0
[  266.193196]  [<c10a15ed>] ? lock_release_non_nested+0x29d/0x2e0
[  266.193196]  [<c1073f35>] ? local_clock+0x65/0x70
[  266.193196]  [<c1178ab1>] ? fget_light+0x371/0x450
[  266.193196]  [<c14eaccb>] sys_sendmsg+0x3b/0x60
[  266.193196]  [<c14eb373>] sys_socketcall+0x283/0x2e0
[  266.193196]  [<c16103e0>] ? restore_all+0xf/0xf
[  266.193196]  [<c1613c90>] ? __do_page_fault+0x4e0/0x4e0
[  266.193196]  [<c12fa548>] ? trace_hardirqs_on_thunk+0xc/0x10
[  266.193196]  [<c1617b8d>] sysenter_do_call+0x12/0x38
[  266.193196] Code: 00 c7 00 00 00 00 00 89 cf 89 c3 c7 40 04 00 00 00 00 89 d6 b9 ff ff ff ff c7 02 00 00 00 00 c7 42 04 00 b
[  266.193196] EIP: [<f8177388>] veth_stats_one.isra.5+0x38/0xd0 [veth] SS:ESP 0068:f47cd840
[  266.193196] CR2: 00000000000002c0
[  266.553774] ---[ end trace fff0ac235458be49 ]---

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: NULL pointer dereference in veth_stats_one

[Eric Dumazet]

On Fri, 2013-01-04 at 10:59 +0000, Tom Parkin wrote:
> Hi list,
> 
> I recently tripped over a NULL pointer dereference in the veth driver.
> I'm running a 3.8.0_rc1 (updated from net-next git tree this morning)
> on an Athlon 64 X2 machine running a 32 bit kernel.  To trigger the
> oops I simply created a veth interface as follows:
> 
>         ip link add name ve0 type veth peer name ve1
> 
> I did a little digging in the git history and I note that veth
> statistics changed a little with commit 2681128f0ced8aa4.  I tried
> reverting that commit in my tree, which made the oops go away again.
> 
> Thanks,
> Tom

Thanks Tom, I'll fix this.

Re: NULL pointer dereference in veth_stats_one

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

On Fri, 2013-01-04 at 07:45 -0800, Eric Dumazet wrote:
> On Fri, 2013-01-04 at 10:59 +0000, Tom Parkin wrote:
> > Hi list,
> > 
> > I recently tripped over a NULL pointer dereference in the veth driver.
> > I'm running a 3.8.0_rc1 (updated from net-next git tree this morning)
> > on an Athlon 64 X2 machine running a 32 bit kernel.  To trigger the
> > oops I simply created a veth interface as follows:
> > 
> >         ip link add name ve0 type veth peer name ve1
> > 
> > I did a little digging in the git history and I note that veth
> > statistics changed a little with commit 2681128f0ced8aa4.  I tried
> > reverting that commit in my tree, which made the oops go away again.
> > 
> > Thanks,
> > Tom
> 
> Thanks Tom, I'll fix this.
> 

Oh well, a last minute change again...

I was fooled by veth_get_ethtool_stats() doing the priv->peer->ifindex
deref without checking.

Here is the fix, thanks !

[PATCH net-next] veth: avoid a NULL deref in veth_stats_one

commit 2681128f0ced8a (veth: extend device features) added a NULL deref
in veth_stats_one(), as veth_get_stats64() was not testing if the peer
device was setup or not.

At init time, we call dev_get_stats() before veth pair is fully setup.

[  178.854758]  [<ffffffffa00f5677>] veth_get_stats64+0x47/0x70 [veth]
[  178.861013]  [<ffffffff814f0a2d>] dev_get_stats+0x6d/0x130
[  178.866486]  [<ffffffff81504efc>] rtnl_fill_ifinfo+0x47c/0x930
[  178.872299]  [<ffffffff81505b93>] rtmsg_ifinfo+0x83/0x100
[  178.877678]  [<ffffffff81505cc6>] rtnl_configure_link+0x76/0xa0
[  178.883580]  [<ffffffffa00f52fa>] veth_newlink+0x16a/0x350 [veth]
[  178.889654]  [<ffffffff815061cc>] rtnl_newlink+0x4dc/0x5e0
[  178.895128]  [<ffffffff81505e1e>] ? rtnl_newlink+0x12e/0x5e0
[  178.900769]  [<ffffffff8150587d>] rtnetlink_rcv_msg+0x11d/0x310
[  178.906669]  [<ffffffff81505760>] ? __rtnl_unlock+0x20/0x20
[  178.912225]  [<ffffffff81521f89>] netlink_rcv_skb+0xa9/0xd0
[  178.917779]  [<ffffffff81502d55>] rtnetlink_rcv+0x25/0x40
[  178.923159]  [<ffffffff815218d1>] netlink_unicast+0x1b1/0x230
[  178.928887]  [<ffffffff81521c4e>] netlink_sendmsg+0x2fe/0x3b0
[  178.934615]  [<ffffffff814dbe22>] sock_sendmsg+0xd2/0xf0

So we must check if peer was setup in veth_get_stats64()

Reported-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/veth.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 8b2e112..bd57213 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -162,15 +162,18 @@ static struct rtnl_link_stats64 *veth_get_stats64(struct net_device *dev,
 						  struct rtnl_link_stats64 *tot)
 {
 	struct veth_priv *priv = netdev_priv(dev);
+	struct net_device *peer = priv->peer;
 	struct pcpu_vstats one;
 
 	tot->tx_dropped = veth_stats_one(&one, dev);
 	tot->tx_bytes = one.bytes;
 	tot->tx_packets = one.packets;
 
-	tot->rx_dropped = veth_stats_one(&one, priv->peer);
-	tot->rx_bytes = one.bytes;
-	tot->rx_packets = one.packets;
+	if (peer) {
+		tot->rx_dropped = veth_stats_one(&one, peer);
+		tot->rx_bytes = one.bytes;
+		tot->rx_packets = one.packets;
+	}
 
 	return tot;
 }

Re: NULL pointer dereference in veth_stats_one

[Ben Hutchings]

On Fri, 2013-01-04 at 08:17 -0800, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> On Fri, 2013-01-04 at 07:45 -0800, Eric Dumazet wrote:
> > On Fri, 2013-01-04 at 10:59 +0000, Tom Parkin wrote:
> > > Hi list,
> > > 
> > > I recently tripped over a NULL pointer dereference in the veth driver.
> > > I'm running a 3.8.0_rc1 (updated from net-next git tree this morning)
> > > on an Athlon 64 X2 machine running a 32 bit kernel.  To trigger the
> > > oops I simply created a veth interface as follows:
> > > 
> > >         ip link add name ve0 type veth peer name ve1
> > > 
> > > I did a little digging in the git history and I note that veth
> > > statistics changed a little with commit 2681128f0ced8aa4.  I tried
> > > reverting that commit in my tree, which made the oops go away again.
> > > 
> > > Thanks,
> > > Tom
> > 
> > Thanks Tom, I'll fix this.
> > 
> 
> Oh well, a last minute change again...
> 
> I was fooled by veth_get_ethtool_stats() doing the priv->peer->ifindex
> deref without checking.
[...]
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -162,15 +162,18 @@ static struct rtnl_link_stats64 *veth_get_stats64(struct net_device *dev,
>  						  struct rtnl_link_stats64 *tot)
>  {
>  	struct veth_priv *priv = netdev_priv(dev);
> +	struct net_device *peer = priv->peer;
>  	struct pcpu_vstats one;
>  
>  	tot->tx_dropped = veth_stats_one(&one, dev);
>  	tot->tx_bytes = one.bytes;
>  	tot->tx_packets = one.packets;
>  
> -	tot->rx_dropped = veth_stats_one(&one, priv->peer);
> -	tot->rx_bytes = one.bytes;
> -	tot->rx_packets = one.packets;
> +	if (peer) {

This possibly needs some memory barriers to properly synchronise with
veth_newlink().  But can you not move initialisation of the peer
pointers before registration of the devices in veth_newlink(), so that
veth_get_stats64() cannot be called before they are initialised?

Ben.

> +		tot->rx_dropped = veth_stats_one(&one, peer);
> +		tot->rx_bytes = one.bytes;
> +		tot->rx_packets = one.packets;
> +	}
>  
>  	return tot;
>  }

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: NULL pointer dereference in veth_stats_one

[Eric Dumazet]

On Fri, 2013-01-04 at 18:17 +0000, Ben Hutchings wrote:

> This possibly needs some memory barriers to properly synchronise with
> veth_newlink().  But can you not move initialisation of the peer
> pointers before registration of the devices in veth_newlink(), so that
> veth_get_stats64() cannot be called before they are initialised?

The ->peer pointer cannot change once set. ( its never cleared )

So the problem would not be in veth_newlink(), but might be in
veth_dellink()

It seems we would have a problem in veth_get_ethtool_stats() already...

More generally, what prevents a get_stats() being called while a
dellink() (-> veth_dev_free() -> free_percpu()) is done ?

(Same thing is done for tunnel/dummy stats percpu data)

Re: NULL pointer dereference in veth_stats_one

[Ben Hutchings]

On Fri, 2013-01-04 at 11:23 -0800, Eric Dumazet wrote:
> On Fri, 2013-01-04 at 18:17 +0000, Ben Hutchings wrote:
> 
> > This possibly needs some memory barriers to properly synchronise with
> > veth_newlink().  But can you not move initialisation of the peer
> > pointers before registration of the devices in veth_newlink(), so that
> > veth_get_stats64() cannot be called before they are initialised?
> 
> The ->peer pointer cannot change once set. ( its never cleared )

We may still need an explicit barrier for data-dependency.

> So the problem would not be in veth_newlink(), but might be in
> veth_dellink()

A lot of things are done in between the unregister_netdevice_queue() and
the actual deletion which are probably sufficient to flush out any calls
to dev_get_stats().  But to make sure, I think we would need some small
amount of shared state that isn't freed until both devices are.

> It seems we would have a problem in veth_get_ethtool_stats() already...

That should be OK because both ethtool operations and the whole process
of interface deletion are serialised by the RTNL lock.

> More generally, what prevents a get_stats() being called while a
> dellink() (-> veth_dev_free() -> free_percpu()) is done ?

Anything calling dev_get_stats() must have a counted or RCU reference to
the device, and netdev_run_todo() waits for those to go away.  For
mutually referencing devices we want a kind of weak reference and we
have no good way to implement those.

Ben.

> (Same thing is done for tunnel/dummy stats percpu data)

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: NULL pointer dereference in veth_stats_one

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

On Fri, 2013-01-04 at 20:25 +0000, Ben Hutchings wrote:

> Anything calling dev_get_stats() must have a counted or RCU reference to
> the device, and netdev_run_todo() waits for those to go away.  For
> mutually referencing devices we want a kind of weak reference and we
> have no good way to implement those.

OK, so to be on the safe side I added RCU barriers/synchro everywhere.

Thanks !

[PATCH v2 net-next] veth: avoid a NULL deref in veth_stats_one

commit 2681128f0ced8a (veth: extend device features) added a NULL deref
in veth_stats_one(), as veth_get_stats64() was not testing if the peer
device was setup or not.

At init time, we call dev_get_stats() before veth pair is fully setup.

[  178.854758]  [<ffffffffa00f5677>] veth_get_stats64+0x47/0x70 [veth]
[  178.861013]  [<ffffffff814f0a2d>] dev_get_stats+0x6d/0x130
[  178.866486]  [<ffffffff81504efc>] rtnl_fill_ifinfo+0x47c/0x930
[  178.872299]  [<ffffffff81505b93>] rtmsg_ifinfo+0x83/0x100
[  178.877678]  [<ffffffff81505cc6>] rtnl_configure_link+0x76/0xa0
[  178.883580]  [<ffffffffa00f52fa>] veth_newlink+0x16a/0x350 [veth]
[  178.889654]  [<ffffffff815061cc>] rtnl_newlink+0x4dc/0x5e0
[  178.895128]  [<ffffffff81505e1e>] ? rtnl_newlink+0x12e/0x5e0
[  178.900769]  [<ffffffff8150587d>] rtnetlink_rcv_msg+0x11d/0x310
[  178.906669]  [<ffffffff81505760>] ? __rtnl_unlock+0x20/0x20
[  178.912225]  [<ffffffff81521f89>] netlink_rcv_skb+0xa9/0xd0
[  178.917779]  [<ffffffff81502d55>] rtnetlink_rcv+0x25/0x40
[  178.923159]  [<ffffffff815218d1>] netlink_unicast+0x1b1/0x230
[  178.928887]  [<ffffffff81521c4e>] netlink_sendmsg+0x2fe/0x3b0
[  178.934615]  [<ffffffff814dbe22>] sock_sendmsg+0xd2/0xf0

So we must check if peer was setup in veth_get_stats64()

As pointed out by Ben Hutchings, priv->peer is missing proper
synchronization. Adding RCU protection is a safe and well documented
way to make sure we don't access about to be freed or already
freed data.

Reported-by: Tom Parkin <tparkin@katalix.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
CC: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/veth.c |   58 +++++++++++++++++++++++++++++--------------
 1 file changed, 40 insertions(+), 18 deletions(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 8b2e112..e778bff 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -32,7 +32,7 @@ struct pcpu_vstats {
 };
 
 struct veth_priv {
-	struct net_device	*peer;
+	struct net_device __rcu	*peer;
 	atomic64_t		dropped;
 };
 
@@ -89,10 +89,10 @@ static int veth_get_sset_count(struct net_device *dev, int sset)
 static void veth_get_ethtool_stats(struct net_device *dev,
 		struct ethtool_stats *stats, u64 *data)
 {
-	struct veth_priv *priv;
+	struct veth_priv *priv = netdev_priv(dev);
+	struct net_device *peer = rtnl_dereference(priv->peer);
 
-	priv = netdev_priv(dev);
-	data[0] = priv->peer->ifindex;
+	data[0] = peer ? peer->ifindex : 0;
 }
 
 static const struct ethtool_ops veth_ethtool_ops = {
@@ -107,9 +107,15 @@ static const struct ethtool_ops veth_ethtool_ops = {
 static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct veth_priv *priv = netdev_priv(dev);
-	struct net_device *rcv = priv->peer;
+	struct net_device *rcv;
 	int length = skb->len;
 
+	rcu_read_lock();
+	rcv = rcu_dereference(priv->peer);
+	if (unlikely(!rcv)) {
+		kfree_skb(skb);
+		goto drop;
+	}
 	/* don't change ip_summed == CHECKSUM_PARTIAL, as that
 	 * will cause bad checksum on forwarded packets
 	 */
@@ -125,9 +131,10 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
 		stats->packets++;
 		u64_stats_update_end(&stats->syncp);
 	} else {
+drop:
 		atomic64_inc(&priv->dropped);
 	}
-
+	rcu_read_unlock();
 	return NETDEV_TX_OK;
 }
 
@@ -162,30 +169,36 @@ static struct rtnl_link_stats64 *veth_get_stats64(struct net_device *dev,
 						  struct rtnl_link_stats64 *tot)
 {
 	struct veth_priv *priv = netdev_priv(dev);
+	struct net_device *peer;
 	struct pcpu_vstats one;
 
 	tot->tx_dropped = veth_stats_one(&one, dev);
 	tot->tx_bytes = one.bytes;
 	tot->tx_packets = one.packets;
 
-	tot->rx_dropped = veth_stats_one(&one, priv->peer);
-	tot->rx_bytes = one.bytes;
-	tot->rx_packets = one.packets;
+	rcu_read_lock();
+	peer = rcu_dereference(priv->peer);
+	if (peer) {
+		tot->rx_dropped = veth_stats_one(&one, peer);
+		tot->rx_bytes = one.bytes;
+		tot->rx_packets = one.packets;
+	}
+	rcu_read_unlock();
 
 	return tot;
 }
 
 static int veth_open(struct net_device *dev)
 {
-	struct veth_priv *priv;
+	struct veth_priv *priv = netdev_priv(dev);
+	struct net_device *peer = rtnl_dereference(priv->peer);
 
-	priv = netdev_priv(dev);
-	if (priv->peer == NULL)
+	if (!peer)
 		return -ENOTCONN;
 
-	if (priv->peer->flags & IFF_UP) {
+	if (peer->flags & IFF_UP) {
 		netif_carrier_on(dev);
-		netif_carrier_on(priv->peer);
+		netif_carrier_on(peer);
 	}
 	return 0;
 }
@@ -195,7 +208,7 @@ static int veth_close(struct net_device *dev)
 	struct veth_priv *priv = netdev_priv(dev);
 
 	netif_carrier_off(dev);
-	netif_carrier_off(priv->peer);
+	netif_carrier_off(rtnl_dereference(priv->peer));
 
 	return 0;
 }
@@ -380,10 +393,10 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
 	 */
 
 	priv = netdev_priv(dev);
-	priv->peer = peer;
+	rcu_assign_pointer(priv->peer, peer);
 
 	priv = netdev_priv(peer);
-	priv->peer = dev;
+	rcu_assign_pointer(priv->peer, dev);
 	return 0;
 
 err_register_dev:
@@ -404,7 +417,16 @@ static void veth_dellink(struct net_device *dev, struct list_head *head)
 	struct net_device *peer;
 
 	priv = netdev_priv(dev);
-	peer = priv->peer;
+	peer = rtnl_dereference(priv->peer);
+
+	/* Note : dellink() is called from default_device_exit_batch(),
+	 * before a rcu_synchronize() point. The devices are guaranteed
+	 * not being freed before one RCU grace period.
+	 */
+	RCU_INIT_POINTER(priv->peer, NULL);
+
+	priv = netdev_priv(peer);
+	RCU_INIT_POINTER(priv->peer, NULL);
 
 	unregister_netdevice_queue(dev, head);
 	unregister_netdevice_queue(peer, head);

Re: NULL pointer dereference in veth_stats_one

[David Miller]

From: Eric Dumazet <erdnetdev@gmail.com>
Date: Fri, 04 Jan 2013 17:42:40 -0800

> From: Eric Dumazet <edumazet@google.com>
 ...
> [PATCH v2 net-next] veth: avoid a NULL deref in veth_stats_one

Applied, thanks Eric.