[PATCH] ath9k_htc: Fix memory leak

[PATCH] ath9k_htc: Fix memory leak

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

SKBs that are allocated in the HTC layer do not have callbacks
registered and hence ended up not being freed, Fix this by freeing
them properly in the TX completion routine.

Cc: <stable@vger.kernel.org>
Reported-by: Larry Finger <Larry.Finger@lwfinger.net>
Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath9k/htc_hst.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/htc_hst.c b/drivers/net/wireless/ath/ath9k/htc_hst.c
index 4a9570d..aac4a40 100644
--- a/drivers/net/wireless/ath/ath9k/htc_hst.c
+++ b/drivers/net/wireless/ath/ath9k/htc_hst.c
@@ -344,6 +344,8 @@ void ath9k_htc_txcompletion_cb(struct htc_target *htc_handle,
 			endpoint->ep_callbacks.tx(endpoint->ep_callbacks.priv,
 						  skb, htc_hdr->endpoint_id,
 						  txok);
+		} else {
+			kfree_skb(skb);
 		}
 	}
 
-- 
1.8.1

Re: [PATCH] ath9k_htc: Fix memory leak

[Larry Finger]

On 01/09/2013 04:37 AM, Sujith Manoharan wrote:
> From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
>
> SKBs that are allocated in the HTC layer do not have callbacks
> registered and hence ended up not being freed, Fix this by freeing
> them properly in the TX completion routine.
>
> Cc: <stable@vger.kernel.org>
> Reported-by: Larry Finger <Larry.Finger@lwfinger.net>
> Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> ---
>   drivers/net/wireless/ath/ath9k/htc_hst.c | 2 ++
>   1 file changed, 2 insertions(+)

Yes, your fix handles the normal case, and you can add a
Tested-by: Larry Finger <Larry.Finger@lwfinger.net>

I still have one remaining question. It is possible for the timeout code in 
htc_config_pipe_credits() to be executed, as shown below:

         time_left = wait_for_completion_timeout(&target->cmd_wait, HZ);
         if (!time_left) {
                 dev_err(target->dev, "HTC credit config timeout\n");
                 return -ETIMEDOUT;
         }

When this timeout happens, is the skb leaked?

Larry

Re: [PATCH] ath9k_htc: Fix memory leak

[Sujith Manoharan]

Larry Finger wrote:
> Yes, your fix handles the normal case, and you can add a
> Tested-by: Larry Finger <Larry.Finger@lwfinger.net>

Thanks.

> I still have one remaining question. It is possible for the timeout code in 
> htc_config_pipe_credits() to be executed, as shown below:
> 
>          time_left = wait_for_completion_timeout(&target->cmd_wait, HZ);
>          if (!time_left) {
>                  dev_err(target->dev, "HTC credit config timeout\n");
>                  return -ETIMEDOUT;
>          }
> 
> When this timeout happens, is the skb leaked?

No, the submitted SKB (via htc_issue_send()) will always have a TX completion
invocation. The actual completion of target->cmd_wait happens via a different
code-path, htc_process_conn_rsp() in ath9k_htc_rx_msg().

Sujith