Re: [PATCH] net/mlx4_en: Fix a memory leak in case of error in 'mlx4_en_init_netdev()'

2018-03-12 Thread Christophe Jaillet 

Le 12/03/2018 à 09:42, Tariq Toukan a écrit :
>
>
> On 12/03/2018 12:45 AM, Christophe JAILLET wrote:
>> If 'kzalloc' fails, we must free some memory before returning.
>>
>> Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings
>> scheme")
>> Signed-off-by: Christophe JAILLET 
>> ---
>>   drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> index 8fc51bc29003..f9db018e858f 100644
>> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
>> @@ -3327,7 +3327,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev
>> *mdev, int port,
>>   if (!priv->tx_cq[t]) {
>>   kfree(priv->tx_ring[t]);
>>   err = -ENOMEM;
>> -goto out;
>> +goto err_free_tx;
>>   }
>>   }
>>   priv->rx_ring_num = prof->rx_ring_num;
>>
>
> Hi Christophe, thanks for spotting this.
>
> However, I think these err_free_tx label and loop are redundant.
> Both tx_ring/tx_cq flows should just goto out, as resources are freed
> later in mlx4_en_destroy_netdev() -> mlx4_en_free_resources().
>

Hi,

I do not agree with you and I think that the patch is relevant.

If 'mlx4_en_init_netdev' fails, the only caller, 'mlx4_en_activate()', 
will set:

mdev->pndev[i] = NULL
(see 
https://elixir.bootlin.com/linux/v4.16-rc5/source/drivers/net/ethernet/mellanox/mlx4/en_main.c#L254)


and 'mlx4_en_destroy_netdev()' is not called in this case.
(see 
https://elixir.bootlin.com/linux/v4.16-rc5/source/drivers/net/ethernet/mellanox/mlx4/en_main.c#L232)


My understanding is that 'mlx4_en_destroy_netdev()' will free resources 
in the normal case but that resources should be freed at allocation time 
if it does not fully succeed.


Best regards,
CJ

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel 
antivirus Avast.
https://www.avast.com/antivirus

Re: [PATCH] net/mlx4_en: Fix a memory leak in case of error in 'mlx4_en_init_netdev()'

2018-03-12 Thread Tariq Toukan 



On 12/03/2018 12:45 AM, Christophe JAILLET wrote:

If 'kzalloc' fails, we must free some memory before returning.

Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
Signed-off-by: Christophe JAILLET 
---
  drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c 
b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 8fc51bc29003..f9db018e858f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3327,7 +3327,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int 
port,
if (!priv->tx_cq[t]) {
kfree(priv->tx_ring[t]);
err = -ENOMEM;
-   goto out;
+   goto err_free_tx;
}
}
priv->rx_ring_num = prof->rx_ring_num;



Hi Christophe, thanks for spotting this.

However, I think these err_free_tx label and loop are redundant.
Both tx_ring/tx_cq flows should just goto out, as resources are freed 
later in mlx4_en_destroy_netdev() -> mlx4_en_free_resources().

[PATCH] net/mlx4_en: Fix a memory leak in case of error in 'mlx4_en_init_netdev()'

2018-03-11 Thread Christophe JAILLET 
If 'kzalloc' fails, we must free some memory before returning.

Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
Signed-off-by: Christophe JAILLET 
---
 drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c 
b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 8fc51bc29003..f9db018e858f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -3327,7 +3327,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int 
port,
if (!priv->tx_cq[t]) {
kfree(priv->tx_ring[t]);
err = -ENOMEM;
-   goto out;
+   goto err_free_tx;
}
}
priv->rx_ring_num = prof->rx_ring_num;
-- 
2.14.1


---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel 
antivirus Avast.
https://www.avast.com/antivirus