On 2018/08/15 10:04, dsah...@kernel.org wrote:
> From: David Ahern <dsah...@gmail.com>
>
> kmemleak reported new suspected memory leaks.
> $ 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]
>
> The allocations in question are veth_alloc_queues for the dev and its peer.
>
> Free the queues on a delete.
>
> Fixes: 638264dc90227 ("veth: Support per queue XDP ring")
> Signed-off-by: David Ahern <dsah...@gmail.com>
> ---
> v2
> - free peer dev queues as well
>
> drivers/net/veth.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index e3202af72df5..2a3ce60631ef 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -1205,6 +1205,7 @@ static void veth_dellink(struct net_device *dev, struct
> list_head *head)
> struct veth_priv *priv;
> struct net_device *peer;
>
> + veth_free_queues(dev);
> priv = netdev_priv(dev);
> peer = rtnl_dereference(priv->peer);
>
> @@ -1216,6 +1217,7 @@ static void veth_dellink(struct net_device *dev, struct
> list_head *head)
> unregister_netdevice_queue(dev, head);
>
> if (peer) {
> + veth_free_queues(peer);
> priv = netdev_priv(peer);
> RCU_INIT_POINTER(priv->peer, NULL);
> unregister_netdevice_queue(peer, head);
Hmm, on second thought this queues need to be freed after veth_close()
to make sure no packet will reference them. That means we need to free
them in .ndo_uninit() or destructor.
(rtnl_delete_link() calls dellink() before unregister_netdevice_many()
which calls dev_close_many() through rollback_registered_many())
Currently veth has destructor veth_dev_free() for vstats, so we can free
queues in the function.
To be in line with vstats, allocation also should be moved to
veth_dev_init().
--
Toshiaki Makita
