2016-04-27 10:43:16

by Haishuang Yan

Subject: [PATCH] veth: Fix potential memory leak in veth_newlink

Free peer netdev when failed to configure peer link or register dev.

Signed-off-by: Haishuang Yan <[email protected]>
---
drivers/net/veth.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index f37a6e6..8bb9fb8 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -472,7 +472,6 @@ err_register_dev:
/* nothing to do */
err_configure_peer:
unregister_netdevice(peer);
- return err;

err_register_peer:
free_netdev(peer);
--
1.8.3.1

2016-04-27 10:57:19

by Nikolay Aleksandrov

Subject: Re: [PATCH] veth: Fix potential memory leak in veth_newlink

On 04/27/2016 12:42 PM, Haishuang Yan wrote:
> Free peer netdev when failed to configure peer link or register dev.
>
> Signed-off-by: Haishuang Yan <[email protected]>
> ---
> drivers/net/veth.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index f37a6e6..8bb9fb8 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -472,7 +472,6 @@ err_register_dev:
> /* nothing to do */
> err_configure_peer:
> unregister_netdevice(peer);
> - return err;
>
> err_register_peer:
> free_netdev(peer);
>

No, it won't leak. unregister_netdevice() will queue it on the todo list and at
the next rtnl unlock the peer device will get freed.
In fact calling it like this you'll hit BUG_ON(dev->reg_state != NETREG_UNREGISTERED) in
free_netdev so I guess you didn't even test your patch.