Re: [PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'

From: Tariq Toukan
Date: Thu May 10 2018 - 11:03:57 EST




On 10/05/2018 5:36 PM, Tariq Toukan wrote:


On 10/05/2018 5:18 PM, Dan Carpenter wrote:
On Thu, May 10, 2018 at 04:38:08PM +0300, Yuval Shaia wrote:
On Thu, May 10, 2018 at 09:02:26AM +0200, Christophe JAILLET wrote:
If an error occurs, 'mlx4_en_destroy_netdev()' is called.
It then calls 'mlx4_en_free_resources()' which does the needed resources
cleanup.

So, doing some explicit kfree in the error handling path would lead to
some double kfree.

Patch make sense but what's bothering me is that mlx4_en_free_resources
loops on the entire array, assuming !priv->tx_ring[t] means entry is
allocated but the existing code does not assume that, see [1]. So i looked
to see where tx_ring array is zeroed and didn't find it.

Am i missing something here.


It's zeroed twice. alloc_etherdev_mqs() allocates zeroed memory and
then we do a memset(priv, 0, sizeof(struct mlx4_en_priv));

regards,
dan carpenter


We do zero (twice) on init, that's right. But I think Yuval's comment is valid in case of the driver went into configuration change, or down/up, that reallocates the rings. I'm double checking this.

Well, the flows in which we need to nullify the tx_rings pointer (if any, I still need to investigate this) is not related to this init function.

Here we're safe.

Anyway, a V2 is already submitted, please use it for your next comments. I think patch is OK.