* segfault after VLAN change
@ 2014-07-01 16:55 Madalin-Cristian Bucur
2014-07-02 7:11 ` Bjørn Mork
2014-07-02 9:25 ` [PATCH net] vlan: free percpu stats in device destructor Eric Dumazet
0 siblings, 2 replies; 8+ messages in thread
From: Madalin-Cristian Bucur @ 2014-07-01 16:55 UTC (permalink / raw)
To: Li RongQing, Eric Dumazet, David S. Miller; +Cc: netdev
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: segfault after VLAN change 2014-07-01 16:55 segfault after VLAN change Madalin-Cristian Bucur @ 2014-07-02 7:11 ` Bjørn Mork 2014-07-02 8:23 ` Eric Dumazet 2014-07-02 9:25 ` [PATCH net] vlan: free percpu stats in device destructor Eric Dumazet 1 sibling, 1 reply; 8+ messages in thread From: Bjørn Mork @ 2014-07-02 7:11 UTC (permalink / raw) To: Madalin-Cristian Bucur; +Cc: Li RongQing, Eric Dumazet, David S. Miller, netdev 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: segfault after VLAN change 2014-07-02 7:11 ` Bjørn Mork @ 2014-07-02 8:23 ` Eric Dumazet 2014-07-02 8:33 ` Eric Dumazet 2014-07-02 10:22 ` Bjørn Mork 0 siblings, 2 replies; 8+ messages in thread From: Eric Dumazet @ 2014-07-02 8:23 UTC (permalink / raw) To: Bjørn Mork Cc: Madalin-Cristian Bucur, Li RongQing, Eric Dumazet, David S. Miller, netdev 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: segfault after VLAN change 2014-07-02 8:23 ` Eric Dumazet @ 2014-07-02 8:33 ` Eric Dumazet 2014-07-02 8:55 ` Madalin-Cristian Bucur 2014-07-02 10:22 ` Bjørn Mork 1 sibling, 1 reply; 8+ messages in thread From: Eric Dumazet @ 2014-07-02 8:33 UTC (permalink / raw) To: Bjørn Mork Cc: Madalin-Cristian Bucur, Li RongQing, Eric Dumazet, David S. Miller, netdev 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); ^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: segfault after VLAN change 2014-07-02 8:33 ` Eric Dumazet @ 2014-07-02 8:55 ` Madalin-Cristian Bucur 0 siblings, 0 replies; 8+ messages in thread From: Madalin-Cristian Bucur @ 2014-07-02 8:55 UTC (permalink / raw) To: Eric Dumazet, Bjørn Mork Cc: Li RongQing, Eric Dumazet, David S. Miller, netdev > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: segfault after VLAN change 2014-07-02 8:23 ` Eric Dumazet 2014-07-02 8:33 ` Eric Dumazet @ 2014-07-02 10:22 ` Bjørn Mork 1 sibling, 0 replies; 8+ messages in thread From: Bjørn Mork @ 2014-07-02 10:22 UTC (permalink / raw) To: Eric Dumazet Cc: Madalin-Cristian Bucur, Li RongQing, Eric Dumazet, David S. Miller, netdev 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net] vlan: free percpu stats in device destructor 2014-07-01 16:55 segfault after VLAN change Madalin-Cristian Bucur 2014-07-02 7:11 ` Bjørn Mork @ 2014-07-02 9:25 ` Eric Dumazet 2014-07-03 0:02 ` David Miller 1 sibling, 1 reply; 8+ messages in thread From: Eric Dumazet @ 2014-07-02 9:25 UTC (permalink / raw) To: Madalin-Cristian Bucur; +Cc: Li RongQing, Eric Dumazet, David S. Miller, netdev 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); ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net] vlan: free percpu stats in device destructor 2014-07-02 9:25 ` [PATCH net] vlan: free percpu stats in device destructor Eric Dumazet @ 2014-07-03 0:02 ` David Miller 0 siblings, 0 replies; 8+ messages in thread From: David Miller @ 2014-07-03 0:02 UTC (permalink / raw) To: eric.dumazet; +Cc: madalin.bucur, roy.qing.li, edumazet, netdev 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-07-03 0:02 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-07-01 16:55 segfault after VLAN change Madalin-Cristian Bucur 2014-07-02 7:11 ` Bjørn Mork 2014-07-02 8:23 ` Eric Dumazet 2014-07-02 8:33 ` Eric Dumazet 2014-07-02 8:55 ` Madalin-Cristian Bucur 2014-07-02 10:22 ` Bjørn Mork 2014-07-02 9:25 ` [PATCH net] vlan: free percpu stats in device destructor Eric Dumazet 2014-07-03 0:02 ` David Miller
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.