diff options
author | Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp> | 2018-08-15 17:07:29 +0900 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2018-08-16 12:22:31 -0700 |
commit | 7797b93b756dabffd1a8db3e7e7b778fb07ef0a6 (patch) | |
tree | 69dec71c0163d9af48235dc545ba9dfaf53e10cb | |
parent | ff93bca769925a2d8fd7f910cdf543d992e17f07 (diff) | |
download | linux-7797b93b756dabffd1a8db3e7e7b778fb07ef0a6.tar.bz2 |
veth: Free queues on link delete
David Ahern reported memory leak in veth.
=======================================================================
$ cat /sys/kernel/debug/kmemleak
unreferenced object 0xffff8800354d5c00 (size 1024):
comm "ip", pid 836, jiffies 4294722952 (age 25.904s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<(____ptrval____)>] kmemleak_alloc+0x70/0x94
[<(____ptrval____)>] slab_post_alloc_hook+0x42/0x52
[<(____ptrval____)>] __kmalloc+0x101/0x142
[<(____ptrval____)>] kmalloc_array.constprop.20+0x1e/0x26 [veth]
[<(____ptrval____)>] veth_newlink+0x147/0x3ac [veth]
...
unreferenced object 0xffff88002e009c00 (size 1024):
comm "ip", pid 836, jiffies 4294722958 (age 25.898s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<(____ptrval____)>] kmemleak_alloc+0x70/0x94
[<(____ptrval____)>] slab_post_alloc_hook+0x42/0x52
[<(____ptrval____)>] __kmalloc+0x101/0x142
[<(____ptrval____)>] kmalloc_array.constprop.20+0x1e/0x26 [veth]
[<(____ptrval____)>] veth_newlink+0x219/0x3ac [veth]
=======================================================================
veth_rq allocated in veth_newlink() was not freed on dellink.
We need to free up them after veth_close() so that any packets will not
reference the queues afterwards. Thus free them in veth_dev_free() in
the same way as freeing stats structure (vstats).
Also move queues allocation to veth_dev_init() to be in line with stats
allocation.
Fixes: 638264dc90227 ("veth: Support per queue XDP ring")
Reported-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Reviewed-by: David Ahern <dsahern@gmail.com>
Tested-by: David Ahern <dsahern@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r-- | drivers/net/veth.c | 70 |
1 files changed, 33 insertions, 37 deletions
diff --git a/drivers/net/veth.c b/drivers/net/veth.c index e3202af72df5..8d679c8b7f25 100644 --- a/drivers/net/veth.c +++ b/drivers/net/veth.c @@ -789,16 +789,48 @@ static int is_valid_veth_mtu(int mtu) return mtu >= ETH_MIN_MTU && mtu <= ETH_MAX_MTU; } +static int veth_alloc_queues(struct net_device *dev) +{ + struct veth_priv *priv = netdev_priv(dev); + int i; + + priv->rq = kcalloc(dev->num_rx_queues, sizeof(*priv->rq), GFP_KERNEL); + if (!priv->rq) + return -ENOMEM; + + for (i = 0; i < dev->num_rx_queues; i++) + priv->rq[i].dev = dev; + + return 0; +} + +static void veth_free_queues(struct net_device *dev) +{ + struct veth_priv *priv = netdev_priv(dev); + + kfree(priv->rq); +} + static int veth_dev_init(struct net_device *dev) { + int err; + dev->vstats = netdev_alloc_pcpu_stats(struct pcpu_vstats); if (!dev->vstats) return -ENOMEM; + + err = veth_alloc_queues(dev); + if (err) { + free_percpu(dev->vstats); + return err; + } + return 0; } static void veth_dev_free(struct net_device *dev) { + veth_free_queues(dev); free_percpu(dev->vstats); } @@ -1040,31 +1072,13 @@ static int veth_validate(struct nlattr *tb[], struct nlattr *data[], return 0; } -static int veth_alloc_queues(struct net_device *dev) -{ - struct veth_priv *priv = netdev_priv(dev); - - priv->rq = kcalloc(dev->num_rx_queues, sizeof(*priv->rq), GFP_KERNEL); - if (!priv->rq) - return -ENOMEM; - - return 0; -} - -static void veth_free_queues(struct net_device *dev) -{ - struct veth_priv *priv = netdev_priv(dev); - - kfree(priv->rq); -} - static struct rtnl_link_ops veth_link_ops; static int veth_newlink(struct net *src_net, struct net_device *dev, struct nlattr *tb[], struct nlattr *data[], struct netlink_ext_ack *extack) { - int err, i; + int err; struct net_device *peer; struct veth_priv *priv; char ifname[IFNAMSIZ]; @@ -1117,12 +1131,6 @@ static int veth_newlink(struct net *src_net, struct net_device *dev, return PTR_ERR(peer); } - err = veth_alloc_queues(peer); - if (err) { - put_net(net); - goto err_peer_alloc_queues; - } - if (!ifmp || !tbp[IFLA_ADDRESS]) eth_hw_addr_random(peer); @@ -1151,10 +1159,6 @@ static int veth_newlink(struct net *src_net, struct net_device *dev, * should be re-allocated */ - err = veth_alloc_queues(dev); - if (err) - goto err_alloc_queues; - if (tb[IFLA_ADDRESS] == NULL) eth_hw_addr_random(dev); @@ -1174,28 +1178,20 @@ static int veth_newlink(struct net *src_net, struct net_device *dev, */ priv = netdev_priv(dev); - for (i = 0; i < dev->real_num_rx_queues; i++) - priv->rq[i].dev = dev; rcu_assign_pointer(priv->peer, peer); priv = netdev_priv(peer); - for (i = 0; i < peer->real_num_rx_queues; i++) - priv->rq[i].dev = peer; rcu_assign_pointer(priv->peer, dev); return 0; err_register_dev: - veth_free_queues(dev); -err_alloc_queues: /* nothing to do */ err_configure_peer: unregister_netdevice(peer); return err; err_register_peer: - veth_free_queues(peer); -err_peer_alloc_queues: free_netdev(peer); return err; } |