segfault after VLAN change

segfault after VLAN change

[Madalin-Cristian Bucur]

Hello,

I've discovered that the commit:

    commit 5a4ae5f6e7d4b2b5a9b8981d513345053e40b6ac
    Author: Li RongQing <roy.qing.li@gmail.com>
    Date:   Mon Apr 21 19:49:08 2014 +0800

    vlan: unnecessary to check if vlan_pcpu_stats is NULL

    if allocating memory for vlan_pcpu_stats failed, the device can not be operated

    Signed-off-by: 
    Cc: Eric Dumazet <edumazet@google.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

is causing a segfault when removing vlan on a e1000 device (reproduces with other devices as well).
Re-adding the check or reverting the patch removes the issue (log below).

-----------------------------------------------------------------------
root@p5040ds:~# vconfig add eth8 1
root@p5040ds:~# vconfig rem eth8.1
Unable to handle kernel paging request for data at address 0x2bc88028
Faulting instruction address: 0xc058e950
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=8 CoreNet Generic
Modules linked in:
CPU: 3 PID: 2167 Comm: vconfig Tainted: G        W     3.16.0-rc3-00346-g65e85bf #2
task: e7264d90 ti: e2c2c000 task.ti: e2c2c000
NIP: c058e950 LR: c058ea30 CTR: c058e900
REGS: e2c2db20 TRAP: 0300   Tainted: G        W      (3.16.0-rc3-00346-g65e85bf)
MSR: 00029002 <CE,EE,ME>  CR: 48000428  XER: 20000000
DEAR: 2bc88028 ESR: 00000000
GPR00: c047299c e2c2dbd0 e7264d90 00000000 2bc88000 00000000 ffffffff 00000000
GPR08: 0000000f 00000000 000000ff 00000000 28000422 10121928 10100000 10100000
GPR16: 10100000 00000000 c07c5968 00000000 00000000 00000000 e2c2dc48 e7838000
GPR24: c07c5bac c07c58a8 e77290cc c07b0000 00000000 c05de6c0 e7838000 e2c2dc48
NIP [c058e950] vlan_dev_get_stats64+0x50/0x170
LR [c058ea30] vlan_dev_get_stats64+0x130/0x170
Call Trace:
[e2c2dbd0] [ffffffea] 0xffffffea (unreliable)
[e2c2dc20] [c047299c] dev_get_stats+0x4c/0x140
[e2c2dc40] [c0488ca8] rtnl_fill_ifinfo+0x3d8/0x960
[e2c2dd70] [c0489f4c] rtmsg_ifinfo+0x6c/0x110
[e2c2dd90] [c04731d4] rollback_registered_many+0x344/0x3b0
[e2c2ddd0] [c047332c] rollback_registered+0x2c/0x50
[e2c2ddf0] [c0476058] unregister_netdevice_queue+0x78/0xf0
[e2c2de00] [c058d800] unregister_vlan_dev+0xc0/0x160
[e2c2de20] [c058e360] vlan_ioctl_handler+0x1c0/0x550
[e2c2de90] [c045d11c] sock_ioctl+0x28c/0x2f0
[e2c2deb0] [c010d070] do_vfs_ioctl+0x90/0x7b0
[e2c2df20] [c010d7d0] SyS_ioctl+0x40/0x80
[e2c2df40] [c000f924] ret_from_syscall+0x0/0x3c
--- Exception: c01 at 0xff164ac
    LR = 0xffb2064
Instruction dump:
7c771b78 83298e20 7c962378 3a600000 3a200000 3a525968 3b185bac 480000e4
54a9103a 7c98482e 81370500 7c892214 <81240028> 712a0001 40c20114 7c2004ac
---[ end trace 7ec90716948b1f12 ]---

Segmentation fault
-----------------------------------------------------------------------

after removing the patch:

-----------------------------------------------------------------------
root@p5040ds:~# vconfig add eth8 1
root@p5040ds:~# vconfig rem eth8.1
root@p5040ds:~#
-----------------------------------------------------------------------

Best regards,
Madalin

Re: segfault after VLAN change

[Bjørn Mork]

Madalin-Cristian Bucur <madalin.bucur@freescale.com> writes:

> Hello,
>
> I've discovered that the commit:
>
>     commit 5a4ae5f6e7d4b2b5a9b8981d513345053e40b6ac
>     Author: Li RongQing <roy.qing.li@gmail.com>
>     Date:   Mon Apr 21 19:49:08 2014 +0800
>
>     vlan: unnecessary to check if vlan_pcpu_stats is NULL
>
>     if allocating memory for vlan_pcpu_stats failed, the device can not be operated
>
>     Signed-off-by: 
>     Cc: Eric Dumazet <edumazet@google.com>
>     Signed-off-by: David S. Miller <davem@davemloft.net>
>
> is causing a segfault when removing vlan on a e1000 device (reproduces with other devices as well).
> Re-adding the check or reverting the patch removes the issue (log below).


Yes, that commit should be reverted.

The commit message makes it clear that only allocation failures were
considered, while a simple grep reveals that there at least one site
where that field is explicitly NULLed:

bjorn@nemi:/usr/local/src/git/linux$ git grep vlan_pcpu_stats net/8021q/
net/8021q/vlan_core.c:  struct vlan_pcpu_stats *rx_stats;
net/8021q/vlan_core.c:  rx_stats = this_cpu_ptr(vlan_dev_priv(vlan_dev)->vlan_pcpu_stats);
net/8021q/vlan_dev.c:           struct vlan_pcpu_stats *stats;
net/8021q/vlan_dev.c:           stats = this_cpu_ptr(vlan->vlan_pcpu_stats);
net/8021q/vlan_dev.c:           this_cpu_inc(vlan->vlan_pcpu_stats->tx_dropped);
net/8021q/vlan_dev.c:   vlan_dev_priv(dev)->vlan_pcpu_stats = netdev_alloc_pcpu_stats(struct vlan_pcpu_stats);
net/8021q/vlan_dev.c:   if (!vlan_dev_priv(dev)->vlan_pcpu_stats)
net/8021q/vlan_dev.c:   free_percpu(vlan->vlan_pcpu_stats);
net/8021q/vlan_dev.c:   vlan->vlan_pcpu_stats = NULL;
net/8021q/vlan_dev.c:   struct vlan_pcpu_stats *p;
net/8021q/vlan_dev.c:           p = per_cpu_ptr(vlan_dev_priv(dev)->vlan_pcpu_stats, i);


Without looking further it seems likely that this is done during
teardown, making the original NULL check necessary.



Bjørn

Re: segfault after VLAN change

[Eric Dumazet]

On Wed, 2014-07-02 at 09:11 +0200, Bjørn Mork wrote:
> Madalin-Cristian Bucur <madalin.bucur@freescale.com> writes:
> 
> > Hello,
> >
> > I've discovered that the commit:
> >
> >     commit 5a4ae5f6e7d4b2b5a9b8981d513345053e40b6ac
> >     Author: Li RongQing <roy.qing.li@gmail.com>
> >     Date:   Mon Apr 21 19:49:08 2014 +0800
> >
> >     vlan: unnecessary to check if vlan_pcpu_stats is NULL
> >
> >     if allocating memory for vlan_pcpu_stats failed, the device can not be operated
> >
> >     Signed-off-by: 
> >     Cc: Eric Dumazet <edumazet@google.com>
> >     Signed-off-by: David S. Miller <davem@davemloft.net>
> >
> > is causing a segfault when removing vlan on a e1000 device (reproduces with other devices as well).
> > Re-adding the check or reverting the patch removes the issue (log below).
> 
> 
> Yes, that commit should be reverted.

Maybe not.

> 
> The commit message makes it clear that only allocation failures were
> considered, while a simple grep reveals that there at least one site
> where that field is explicitly NULLed:
> 
> bjorn@nemi:/usr/local/src/git/linux$ git grep vlan_pcpu_stats net/8021q/
> net/8021q/vlan_core.c:  struct vlan_pcpu_stats *rx_stats;
> net/8021q/vlan_core.c:  rx_stats = this_cpu_ptr(vlan_dev_priv(vlan_dev)->vlan_pcpu_stats);
> net/8021q/vlan_dev.c:           struct vlan_pcpu_stats *stats;
> net/8021q/vlan_dev.c:           stats = this_cpu_ptr(vlan->vlan_pcpu_stats);
> net/8021q/vlan_dev.c:           this_cpu_inc(vlan->vlan_pcpu_stats->tx_dropped);
> net/8021q/vlan_dev.c:   vlan_dev_priv(dev)->vlan_pcpu_stats = netdev_alloc_pcpu_stats(struct vlan_pcpu_stats);
> net/8021q/vlan_dev.c:   if (!vlan_dev_priv(dev)->vlan_pcpu_stats)
> net/8021q/vlan_dev.c:   free_percpu(vlan->vlan_pcpu_stats);
> net/8021q/vlan_dev.c:   vlan->vlan_pcpu_stats = NULL;
> net/8021q/vlan_dev.c:   struct vlan_pcpu_stats *p;
> net/8021q/vlan_dev.c:           p = per_cpu_ptr(vlan_dev_priv(dev)->vlan_pcpu_stats, i);
> 
> 
> Without looking further it seems likely that this is done during
> teardown, making the original NULL check necessary.

Then the teardown is not properly done.

Sure a 'revert' helps, but the real bug should be fixed.

The freeing of the percpu structure should happen from
dev->destructor(), not from ndo_uninit

Re: segfault after VLAN change

[Eric Dumazet]

On Wed, 2014-07-02 at 01:23 -0700, Eric Dumazet wrote:
> On Wed, 2014-07-02 at 09:11 +0200, Bjørn Mork wrote:
> > Madalin-Cristian Bucur <madalin.bucur@freescale.com> writes:
> > 
> > > Hello,
> > >
> > > I've discovered that the commit:
> > >
> > >     commit 5a4ae5f6e7d4b2b5a9b8981d513345053e40b6ac
> > >     Author: Li RongQing <roy.qing.li@gmail.com>
> > >     Date:   Mon Apr 21 19:49:08 2014 +0800
> > >
> > >     vlan: unnecessary to check if vlan_pcpu_stats is NULL
> > >
> > >     if allocating memory for vlan_pcpu_stats failed, the device can not be operated
> > >
> > >     Signed-off-by: 
> > >     Cc: Eric Dumazet <edumazet@google.com>
> > >     Signed-off-by: David S. Miller <davem@davemloft.net>
> > >
> > > is causing a segfault when removing vlan on a e1000 device (reproduces with other devices as well).
> > > Re-adding the check or reverting the patch removes the issue (log below).
> > 
> > 
> > Yes, that commit should be reverted.
> 
> Maybe not.
> 
> > 
> > The commit message makes it clear that only allocation failures were
> > considered, while a simple grep reveals that there at least one site
> > where that field is explicitly NULLed:
> > 
> > bjorn@nemi:/usr/local/src/git/linux$ git grep vlan_pcpu_stats net/8021q/
> > net/8021q/vlan_core.c:  struct vlan_pcpu_stats *rx_stats;
> > net/8021q/vlan_core.c:  rx_stats = this_cpu_ptr(vlan_dev_priv(vlan_dev)->vlan_pcpu_stats);
> > net/8021q/vlan_dev.c:           struct vlan_pcpu_stats *stats;
> > net/8021q/vlan_dev.c:           stats = this_cpu_ptr(vlan->vlan_pcpu_stats);
> > net/8021q/vlan_dev.c:           this_cpu_inc(vlan->vlan_pcpu_stats->tx_dropped);
> > net/8021q/vlan_dev.c:   vlan_dev_priv(dev)->vlan_pcpu_stats = netdev_alloc_pcpu_stats(struct vlan_pcpu_stats);
> > net/8021q/vlan_dev.c:   if (!vlan_dev_priv(dev)->vlan_pcpu_stats)
> > net/8021q/vlan_dev.c:   free_percpu(vlan->vlan_pcpu_stats);
> > net/8021q/vlan_dev.c:   vlan->vlan_pcpu_stats = NULL;
> > net/8021q/vlan_dev.c:   struct vlan_pcpu_stats *p;
> > net/8021q/vlan_dev.c:           p = per_cpu_ptr(vlan_dev_priv(dev)->vlan_pcpu_stats, i);
> > 
> > 
> > Without looking further it seems likely that this is done during
> > teardown, making the original NULL check necessary.
> 
> Then the teardown is not properly done.
> 
> Sure a 'revert' helps, but the real bug should be fixed.
> 
> The freeing of the percpu structure should happen from
> dev->destructor(), not from ndo_uninit
> 

Please try the following patch :

 net/8021q/vlan_dev.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index ad2ac3c00398..dd11f612e03e 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -627,8 +627,6 @@ static void vlan_dev_uninit(struct net_device *dev)
 	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
 	int i;
 
-	free_percpu(vlan->vlan_pcpu_stats);
-	vlan->vlan_pcpu_stats = NULL;
 	for (i = 0; i < ARRAY_SIZE(vlan->egress_priority_map); i++) {
 		while ((pm = vlan->egress_priority_map[i]) != NULL) {
 			vlan->egress_priority_map[i] = pm->next;
@@ -785,6 +783,15 @@ static const struct net_device_ops vlan_netdev_ops = {
 	.ndo_get_lock_subclass  = vlan_dev_get_lock_subclass,
 };
 
+static void vlan_dev_free(struct net_device *dev)
+{
+	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
+
+	free_percpu(vlan->vlan_pcpu_stats);
+	vlan->vlan_pcpu_stats = NULL;
+	free_netdev(dev);
+}
+
 void vlan_setup(struct net_device *dev)
 {
 	ether_setup(dev);
@@ -794,7 +801,7 @@ void vlan_setup(struct net_device *dev)
 	dev->tx_queue_len	= 0;
 
 	dev->netdev_ops		= &vlan_netdev_ops;
-	dev->destructor		= free_netdev;
+	dev->destructor		= vlan_dev_free;
 	dev->ethtool_ops	= &vlan_ethtool_ops;
 
 	memset(dev->broadcast, 0, ETH_ALEN);

RE: segfault after VLAN change

[Madalin-Cristian Bucur]

> On Wed, 2014-07-02 at 01:23 -0700, Eric Dumazet wrote:
> > On Wed, 2014-07-02 at 09:11 +0200, Bjørn Mork wrote:
> > > Madalin-Cristian Bucur <madalin.bucur@freescale.com> writes:
> > >
> > > > Hello,
> > > >
> > > > I've discovered that the commit:
> > > >
> > > >     commit 5a4ae5f6e7d4b2b5a9b8981d513345053e40b6ac
> > > >     Author: Li RongQing <roy.qing.li@gmail.com>
> > > >     Date:   Mon Apr 21 19:49:08 2014 +0800
> > > >
> > > >     vlan: unnecessary to check if vlan_pcpu_stats is NULL
> > > >
> > > >     if allocating memory for vlan_pcpu_stats failed, the device can
> not be operated
> > > >
> > > >     Signed-off-by:
> > > >     Cc: Eric Dumazet <edumazet@google.com>
> > > >     Signed-off-by: David S. Miller <davem@davemloft.net>
> > > >
> > > > is causing a segfault when removing vlan on a e1000 device
> (reproduces with other devices as well).
> > > > Re-adding the check or reverting the patch removes the issue (log
> below).
> > >
> > >
> > > Yes, that commit should be reverted.
> >
> > Maybe not.
> >
> > >
> > > The commit message makes it clear that only allocation failures were
> > > considered, while a simple grep reveals that there at least one site
> > > where that field is explicitly NULLed:
> > >
> > > bjorn@nemi:/usr/local/src/git/linux$ git grep vlan_pcpu_stats
> net/8021q/
> > > net/8021q/vlan_core.c:  struct vlan_pcpu_stats *rx_stats;
> > > net/8021q/vlan_core.c:  rx_stats =
> this_cpu_ptr(vlan_dev_priv(vlan_dev)->vlan_pcpu_stats);
> > > net/8021q/vlan_dev.c:           struct vlan_pcpu_stats *stats;
> > > net/8021q/vlan_dev.c:           stats = this_cpu_ptr(vlan-
> >vlan_pcpu_stats);
> > > net/8021q/vlan_dev.c:           this_cpu_inc(vlan->vlan_pcpu_stats-
> >tx_dropped);
> > > net/8021q/vlan_dev.c:   vlan_dev_priv(dev)->vlan_pcpu_stats =
> netdev_alloc_pcpu_stats(struct vlan_pcpu_stats);
> > > net/8021q/vlan_dev.c:   if (!vlan_dev_priv(dev)->vlan_pcpu_stats)
> > > net/8021q/vlan_dev.c:   free_percpu(vlan->vlan_pcpu_stats);
> > > net/8021q/vlan_dev.c:   vlan->vlan_pcpu_stats = NULL;
> > > net/8021q/vlan_dev.c:   struct vlan_pcpu_stats *p;
> > > net/8021q/vlan_dev.c:           p = per_cpu_ptr(vlan_dev_priv(dev)-
> >vlan_pcpu_stats, i);
> > >
> > >
> > > Without looking further it seems likely that this is done during
> > > teardown, making the original NULL check necessary.
> >
> > Then the teardown is not properly done.
> >
> > Sure a 'revert' helps, but the real bug should be fixed.
> >
> > The freeing of the percpu structure should happen from
> > dev->destructor(), not from ndo_uninit
> >
> 
> Please try the following patch :
> 
>  net/8021q/vlan_dev.c |   13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
> index ad2ac3c00398..dd11f612e03e 100644
> --- a/net/8021q/vlan_dev.c
> +++ b/net/8021q/vlan_dev.c
> @@ -627,8 +627,6 @@ static void vlan_dev_uninit(struct net_device *dev)
>  	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
>  	int i;
> 
> -	free_percpu(vlan->vlan_pcpu_stats);
> -	vlan->vlan_pcpu_stats = NULL;
>  	for (i = 0; i < ARRAY_SIZE(vlan->egress_priority_map); i++) {
>  		while ((pm = vlan->egress_priority_map[i]) != NULL) {
>  			vlan->egress_priority_map[i] = pm->next;
> @@ -785,6 +783,15 @@ static const struct net_device_ops vlan_netdev_ops =
> {
>  	.ndo_get_lock_subclass  = vlan_dev_get_lock_subclass,
>  };
> 
> +static void vlan_dev_free(struct net_device *dev)
> +{
> +	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
> +
> +	free_percpu(vlan->vlan_pcpu_stats);
> +	vlan->vlan_pcpu_stats = NULL;
> +	free_netdev(dev);
> +}
> +
>  void vlan_setup(struct net_device *dev)
>  {
>  	ether_setup(dev);
> @@ -794,7 +801,7 @@ void vlan_setup(struct net_device *dev)
>  	dev->tx_queue_len	= 0;
> 
>  	dev->netdev_ops		= &vlan_netdev_ops;
> -	dev->destructor		= free_netdev;
> +	dev->destructor		= vlan_dev_free;
>  	dev->ethtool_ops	= &vlan_ethtool_ops;
> 
>  	memset(dev->broadcast, 0, ETH_ALEN);
> 

Your patch works as expected, no segfault when removing the VLAN.

Best regards,
Madalin

[PATCH net] vlan: free percpu stats in device destructor

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Madalin-Cristian reported crashs happening after a recent commit
(5a4ae5f6e7d4 "vlan: unnecessary to check if vlan_pcpu_stats is NULL")

-----------------------------------------------------------------------
root@p5040ds:~# vconfig add eth8 1
root@p5040ds:~# vconfig rem eth8.1
Unable to handle kernel paging request for data at address 0x2bc88028
Faulting instruction address: 0xc058e950
Oops: Kernel access of bad area, sig: 11 [#1]
SMP NR_CPUS=8 CoreNet Generic
Modules linked in:
CPU: 3 PID: 2167 Comm: vconfig Tainted: G        W     3.16.0-rc3-00346-g65e85bf #2
task: e7264d90 ti: e2c2c000 task.ti: e2c2c000
NIP: c058e950 LR: c058ea30 CTR: c058e900
REGS: e2c2db20 TRAP: 0300   Tainted: G        W      (3.16.0-rc3-00346-g65e85bf)
MSR: 00029002 <CE,EE,ME>  CR: 48000428  XER: 20000000
DEAR: 2bc88028 ESR: 00000000
GPR00: c047299c e2c2dbd0 e7264d90 00000000 2bc88000 00000000 ffffffff 00000000
GPR08: 0000000f 00000000 000000ff 00000000 28000422 10121928 10100000 10100000
GPR16: 10100000 00000000 c07c5968 00000000 00000000 00000000 e2c2dc48 e7838000
GPR24: c07c5bac c07c58a8 e77290cc c07b0000 00000000 c05de6c0 e7838000 e2c2dc48
NIP [c058e950] vlan_dev_get_stats64+0x50/0x170
LR [c058ea30] vlan_dev_get_stats64+0x130/0x170
Call Trace:
[e2c2dbd0] [ffffffea] 0xffffffea (unreliable)
[e2c2dc20] [c047299c] dev_get_stats+0x4c/0x140
[e2c2dc40] [c0488ca8] rtnl_fill_ifinfo+0x3d8/0x960
[e2c2dd70] [c0489f4c] rtmsg_ifinfo+0x6c/0x110
[e2c2dd90] [c04731d4] rollback_registered_many+0x344/0x3b0
[e2c2ddd0] [c047332c] rollback_registered+0x2c/0x50
[e2c2ddf0] [c0476058] unregister_netdevice_queue+0x78/0xf0
[e2c2de00] [c058d800] unregister_vlan_dev+0xc0/0x160
[e2c2de20] [c058e360] vlan_ioctl_handler+0x1c0/0x550
[e2c2de90] [c045d11c] sock_ioctl+0x28c/0x2f0
[e2c2deb0] [c010d070] do_vfs_ioctl+0x90/0x7b0
[e2c2df20] [c010d7d0] SyS_ioctl+0x40/0x80
[e2c2df40] [c000f924] ret_from_syscall+0x0/0x3c

Fix this problem by freeing percpu stats from dev->destructor() instead
of ndo_uninit()

Reported-by: Madalin-Cristian Bucur <madalin.bucur@freescale.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Tested-by: Madalin-Cristian Bucur <madalin.bucur@freescale.com>
Fixes: 5a4ae5f6e7d4 ("vlan: unnecessary to check if vlan_pcpu_stats is NULL")
Cc: Li RongQing <roy.qing.li@gmail.com>
---
 net/8021q/vlan_dev.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index ad2ac3c00398..dd11f612e03e 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -627,8 +627,6 @@ static void vlan_dev_uninit(struct net_device *dev)
 	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
 	int i;
 
-	free_percpu(vlan->vlan_pcpu_stats);
-	vlan->vlan_pcpu_stats = NULL;
 	for (i = 0; i < ARRAY_SIZE(vlan->egress_priority_map); i++) {
 		while ((pm = vlan->egress_priority_map[i]) != NULL) {
 			vlan->egress_priority_map[i] = pm->next;
@@ -785,6 +783,15 @@ static const struct net_device_ops vlan_netdev_ops = {
 	.ndo_get_lock_subclass  = vlan_dev_get_lock_subclass,
 };
 
+static void vlan_dev_free(struct net_device *dev)
+{
+	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
+
+	free_percpu(vlan->vlan_pcpu_stats);
+	vlan->vlan_pcpu_stats = NULL;
+	free_netdev(dev);
+}
+
 void vlan_setup(struct net_device *dev)
 {
 	ether_setup(dev);
@@ -794,7 +801,7 @@ void vlan_setup(struct net_device *dev)
 	dev->tx_queue_len	= 0;
 
 	dev->netdev_ops		= &vlan_netdev_ops;
-	dev->destructor		= free_netdev;
+	dev->destructor		= vlan_dev_free;
 	dev->ethtool_ops	= &vlan_ethtool_ops;
 
 	memset(dev->broadcast, 0, ETH_ALEN);

Re: segfault after VLAN change

[Bjørn Mork]

Eric Dumazet <eric.dumazet@gmail.com> writes:

> Sure a 'revert' helps, but the real bug should be fixed.
>
> The freeing of the percpu structure should happen from
> dev->destructor(), not from ndo_uninit

That makes sense.  Thanks for fixing this.


Bjørn

Re: [PATCH net] vlan: free percpu stats in device destructor

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 02 Jul 2014 02:25:15 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> Madalin-Cristian reported crashs happening after a recent commit
> (5a4ae5f6e7d4 "vlan: unnecessary to check if vlan_pcpu_stats is NULL")
 ...
> Fix this problem by freeing percpu stats from dev->destructor() instead
> of ndo_uninit()
> 
> Reported-by: Madalin-Cristian Bucur <madalin.bucur@freescale.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Tested-by: Madalin-Cristian Bucur <madalin.bucur@freescale.com>
> Fixes: 5a4ae5f6e7d4 ("vlan: unnecessary to check if vlan_pcpu_stats is NULL")

Applied and queued up for -stable, thanks Eric.