Re: [PATCH 2/2] ath6kl: Fix potential memory leak in ath6kl_tx_complete()

[Kalle Valo <kvalo@qca.qualcomm.com>]

On 08/13/2012 04:48 PM, Vasanthakumar Thiagarajan wrote:
> We bail out from ath6kl_tx_complete() if any of the sanity
> checks on skb and ath6kl_cookie fails. By doing this we
> potentially leak few remaining buffers in packet_queue.
> Make sure to proceed processing the remaining buffers
> as well. This issue is found during code review.
> 
> Reported-by: Wang yufeng <yufengw@qca.qualcomm.com>
> Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
> ---
>  drivers/net/wireless/ath/ath6kl/txrx.c |   25 ++++++++++++++-----------
>  1 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath6kl/txrx.c b/drivers/net/wireless/ath/ath6kl/txrx.c
> index aab8251..4b26ba0 100644
> --- a/drivers/net/wireless/ath/ath6kl/txrx.c
> +++ b/drivers/net/wireless/ath/ath6kl/txrx.c
> @@ -698,21 +698,29 @@ void ath6kl_tx_complete(struct htc_target *target,
>  		list_del(&packet->list);
>  
>  		ath6kl_cookie = (struct ath6kl_cookie *)packet->pkt_cntxt;
> -		if (!ath6kl_cookie)
> -			goto fatal;
> +		if (!ath6kl_cookie) {
> +			WARN_ON(1);
> +			continue;
> +		}

Please use WARN_ON_ONCE() to avoid excess log messages (as this is in
data path) and put the WARN_ON() inside if:

if (WARN_ON_ONCE(!ath6kl_cookie))
	continue;

> +		if (!skb || !skb->data) {
> +			WARN_ON(1);
> +			dev_kfree_skb(skb);
> +			ath6kl_free_cookie(ar, ath6kl_cookie);
> +			continue;
> +		}

Same here.

Kalle