Re: [PATCH net 2/2] ibmveth: fix TX LTB and filter unwind on open-fail

[Dave Marquardt <davemarq@linux.ibm.com>]

Mingming Cao <mmc@linux.ibm.com> writes:

> out_free_buffer_pools reuses open()'s loop index for the TX LTB
> walk. After the pool unwind i is -1, so the TX buffers allocated
> earlier are leaked. request_irq() failure has the same leak.
>
> The TX-fail goto on that walk also skips dma_unmap of the filter
> list (pre-existing since the LTB was added). Rewrite the TX walk
> to real_num_tx_queues with a pointer check, and unmap the filter
> list before those frees.
>
> Fixes: d926793c1de9 ("ibmveth: Implement multi queue on xmit")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
> ---
>  drivers/net/ethernet/ibm/ibmveth.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
> index 3e44f6b714d4..abebdb1fc262 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c
> @@ -553,10 +553,15 @@ static int ibmveth_rxq_harvest_buffer(struct ibmveth_adapter *adapter,
>  
>  static void ibmveth_free_tx_ltb(struct ibmveth_adapter *adapter, int idx)
>  {
> +	void *ptr = adapter->tx_ltb_ptr[idx];
> +
> +	if (!ptr)
> +		return;
> +
> +	adapter->tx_ltb_ptr[idx] = NULL;
>  	dma_unmap_single(&adapter->vdev->dev, adapter->tx_ltb_dma[idx],
>  			 adapter->tx_ltb_size, DMA_TO_DEVICE);
> -	kfree(adapter->tx_ltb_ptr[idx]);
> -	adapter->tx_ltb_ptr[idx] = NULL;
> +	kfree(ptr);
>  }
>  
>  static int ibmveth_allocate_tx_ltb(struct ibmveth_adapter *adapter, int idx)
> @@ -667,7 +672,7 @@ static int ibmveth_open(struct net_device *netdev)
>  
>  	for (i = 0; i < netdev->real_num_tx_queues; i++) {
>  		if (ibmveth_allocate_tx_ltb(adapter, i))
> -			goto out_free_tx_ltb;
> +			goto out_unmap_filter_list;
>  	}
>  
>  	adapter->rx_queue.index = 0;
> @@ -745,10 +750,8 @@ static int ibmveth_open(struct net_device *netdev)
>  	dma_unmap_single(dev, adapter->filter_list_dma, 4096,
>  			 DMA_BIDIRECTIONAL);
>  
> -out_free_tx_ltb:
> -	while (--i >= 0) {
> +	for (i = netdev->real_num_tx_queues - 1; i >= 0; i--)
>  		ibmveth_free_tx_ltb(adapter, i);
> -	}
>  
>  out_unmap_buffer_list:
>  	dma_unmap_single(dev, adapter->buffer_list_dma, 4096,

Reviewed-by: Dave Marquardt <davemarq@linux.ibm.com>