[PATCH] ath9k_htc: Fix few possible memory leaks

[PATCH] ath9k_htc: Fix few possible memory leaks

[Mohammed Shafi Shajakhan]

From: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>

still there are few other memory leaks which will be fixed
very soon

Cc: Rajkumar Manoharan <rmanohar@qca.qualcomm.com>
Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
 drivers/net/wireless/ath/ath9k/htc_hst.c |   13 ++++++++-----
 drivers/net/wireless/ath/ath9k/wmi.c     |    3 ++-
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/htc_hst.c b/drivers/net/wireless/ath/ath9k/htc_hst.c
index 1b90ed8..e435c9b 100644
--- a/drivers/net/wireless/ath/ath9k/htc_hst.c
+++ b/drivers/net/wireless/ath/ath9k/htc_hst.c
@@ -169,13 +169,14 @@ static int htc_config_pipe_credits(struct htc_target *target)
 	time_left = wait_for_completion_timeout(&target->cmd_wait, HZ);
 	if (!time_left) {
 		dev_err(target->dev, "HTC credit config timeout\n");
-		return -ETIMEDOUT;
+		ret = -ETIMEDOUT;
+		goto err;
 	}
 
 	return 0;
 err:
 	kfree_skb(skb);
-	return -EINVAL;
+	return ret;
 }
 
 static int htc_setup_complete(struct htc_target *target)
@@ -204,14 +205,15 @@ static int htc_setup_complete(struct htc_target *target)
 	time_left = wait_for_completion_timeout(&target->cmd_wait, HZ);
 	if (!time_left) {
 		dev_err(target->dev, "HTC start timeout\n");
-		return -ETIMEDOUT;
+		ret = -ETIMEDOUT;
+		goto err;
 	}
 
 	return 0;
 
 err:
 	kfree_skb(skb);
-	return -EINVAL;
+	return ret;
 }
 
 /* HTC APIs */
@@ -276,7 +278,8 @@ int htc_connect_service(struct htc_target *target,
 	if (!time_left) {
 		dev_err(target->dev, "Service connection timeout for: %d\n",
 			service_connreq->service_id);
-		return -ETIMEDOUT;
+		ret = -ETIMEDOUT;
+		goto err;
 	}
 
 	*conn_rsp_epid = target->conn_rsp_epid;
diff --git a/drivers/net/wireless/ath/ath9k/wmi.c b/drivers/net/wireless/ath/ath9k/wmi.c
index 35422fc..50d901d 100644
--- a/drivers/net/wireless/ath/ath9k/wmi.c
+++ b/drivers/net/wireless/ath/ath9k/wmi.c
@@ -334,7 +334,8 @@ int ath9k_wmi_cmd(struct wmi *wmi, enum wmi_cmd_id cmd_id,
 			"Timeout waiting for WMI command: %s\n",
 			wmi_cmd_to_name(cmd_id));
 		mutex_unlock(&wmi->op_mutex);
-		return -ETIMEDOUT;
+		ret = -ETIMEDOUT;
+		goto out;
 	}
 
 	mutex_unlock(&wmi->op_mutex);
-- 
1.7.0.4

Re: [PATCH] ath9k_htc: Fix few possible memory leaks

[Vasanthakumar Thiagarajan]

On Sun, Aug 21, 2011 at 09:24:45PM +0530, Mohammed Shafi Shajakhan wrote:
> From: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>
> 
> still there are few other memory leaks which will be fixed
> very soon
> 
> Cc: Rajkumar Manoharan <rmanohar@qca.qualcomm.com>
> Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
>  drivers/net/wireless/ath/ath9k/htc_hst.c |   13 ++++++++-----
>  drivers/net/wireless/ath/ath9k/wmi.c     |    3 ++-
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/htc_hst.c b/drivers/net/wireless/ath/ath9k/htc_hst.c
> index 1b90ed8..e435c9b 100644
> --- a/drivers/net/wireless/ath/ath9k/htc_hst.c
> +++ b/drivers/net/wireless/ath/ath9k/htc_hst.c
> @@ -169,13 +169,14 @@ static int htc_config_pipe_credits(struct htc_target *target)
>  	time_left = wait_for_completion_timeout(&target->cmd_wait, HZ);
>  	if (!time_left) {
>  		dev_err(target->dev, "HTC credit config timeout\n");
> -		return -ETIMEDOUT;
> +		ret = -ETIMEDOUT;
> +		goto err;

Are you sure we have to free the skb upon timeout?. It looks like
it is already taken care in ath9k_htc_txcompletion_cb(). The same
may apply to your other changes as well.

Vasanth

Re: [PATCH] ath9k_htc: Fix few possible memory leaks

[mohammed]

On Monday 22 August 2011 10:59 AM, Vasanthakumar Thiagarajan wrote:
> On Sun, Aug 21, 2011 at 09:24:45PM +0530, Mohammed Shafi Shajakhan wrote:
>> From: Mohammed Shafi Shajakhan<mohammed@qca.qualcomm.com>
>>
>> still there are few other memory leaks which will be fixed
>> very soon
>>
>> Cc: Rajkumar Manoharan<rmanohar@qca.qualcomm.com>
>> Signed-off-by: Mohammed Shafi Shajakhan<mohammed@qca.qualcomm.com>
>> Signed-off-by: Larry Finger<Larry.Finger@lwfinger.net>
>> ---
>>   drivers/net/wireless/ath/ath9k/htc_hst.c |   13 ++++++++-----
>>   drivers/net/wireless/ath/ath9k/wmi.c     |    3 ++-
>>   2 files changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/htc_hst.c b/drivers/net/wireless/ath/ath9k/htc_hst.c
>> index 1b90ed8..e435c9b 100644
>> --- a/drivers/net/wireless/ath/ath9k/htc_hst.c
>> +++ b/drivers/net/wireless/ath/ath9k/htc_hst.c
>> @@ -169,13 +169,14 @@ static int htc_config_pipe_credits(struct htc_target *target)
>>   	time_left = wait_for_completion_timeout(&target->cmd_wait, HZ);
>>   	if (!time_left) {
>>   		dev_err(target->dev, "HTC credit config timeout\n");
>> -		return -ETIMEDOUT;
>> +		ret = -ETIMEDOUT;
>> +		goto err;
>
> Are you sure we have to free the skb upon timeout?. It looks like
> it is already taken care in ath9k_htc_txcompletion_cb(). The same
> may apply to your other changes as well.


Vasanth thanks for your review, I haven't and not aware of that code 
flow/callback. I will check into that.

thanks,
shafi

>
> Vasanth

Re: [PATCH] ath9k_htc: Fix few possible memory leaks

[Larry Finger]

On 08/22/2011 08:56 AM, mohammed wrote:
>
>
> Vasanth thanks for your review, I haven't and not aware of that code
> flow/callback. I will check into that.
>

This patch has helped a lot, but after about 12 hours of running, I still have 
18 leaks found. I will try to sort them out.

Larry

Re: [PATCH] ath9k_htc: Fix few possible memory leaks

[mohammed]

On Monday 22 August 2011 10:18 PM, Larry Finger wrote:
> On 08/22/2011 08:56 AM, mohammed wrote:
>>
>>
>> Vasanth thanks for your review, I haven't and not aware of that code
>> flow/callback. I will check into that.
>>
>
> This patch has helped a lot, but after about 12 hours of running, I
> still have 18 leaks found. I will try to sort them out.

Hi Larry,

thanks for your help, we will also try to find a suitable fix for this.

thanks,
shafi

Re: [PATCH] ath9k_htc: Fix few possible memory leaks

[John W. Linville]

Is this intended for 3.1?  Should it be Cc: stable@kernel.org as well?

On Sun, Aug 21, 2011 at 09:24:45PM +0530, Mohammed Shafi Shajakhan wrote:
> From: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>
> 
> still there are few other memory leaks which will be fixed
> very soon
> 
> Cc: Rajkumar Manoharan <rmanohar@qca.qualcomm.com>
> Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
>  drivers/net/wireless/ath/ath9k/htc_hst.c |   13 ++++++++-----
>  drivers/net/wireless/ath/ath9k/wmi.c     |    3 ++-
>  2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/htc_hst.c b/drivers/net/wireless/ath/ath9k/htc_hst.c
> index 1b90ed8..e435c9b 100644
> --- a/drivers/net/wireless/ath/ath9k/htc_hst.c
> +++ b/drivers/net/wireless/ath/ath9k/htc_hst.c
> @@ -169,13 +169,14 @@ static int htc_config_pipe_credits(struct htc_target *target)
>  	time_left = wait_for_completion_timeout(&target->cmd_wait, HZ);
>  	if (!time_left) {
>  		dev_err(target->dev, "HTC credit config timeout\n");
> -		return -ETIMEDOUT;
> +		ret = -ETIMEDOUT;
> +		goto err;
>  	}
>  
>  	return 0;
>  err:
>  	kfree_skb(skb);
> -	return -EINVAL;
> +	return ret;
>  }
>  
>  static int htc_setup_complete(struct htc_target *target)
> @@ -204,14 +205,15 @@ static int htc_setup_complete(struct htc_target *target)
>  	time_left = wait_for_completion_timeout(&target->cmd_wait, HZ);
>  	if (!time_left) {
>  		dev_err(target->dev, "HTC start timeout\n");
> -		return -ETIMEDOUT;
> +		ret = -ETIMEDOUT;
> +		goto err;
>  	}
>  
>  	return 0;
>  
>  err:
>  	kfree_skb(skb);
> -	return -EINVAL;
> +	return ret;
>  }
>  
>  /* HTC APIs */
> @@ -276,7 +278,8 @@ int htc_connect_service(struct htc_target *target,
>  	if (!time_left) {
>  		dev_err(target->dev, "Service connection timeout for: %d\n",
>  			service_connreq->service_id);
> -		return -ETIMEDOUT;
> +		ret = -ETIMEDOUT;
> +		goto err;
>  	}
>  
>  	*conn_rsp_epid = target->conn_rsp_epid;
> diff --git a/drivers/net/wireless/ath/ath9k/wmi.c b/drivers/net/wireless/ath/ath9k/wmi.c
> index 35422fc..50d901d 100644
> --- a/drivers/net/wireless/ath/ath9k/wmi.c
> +++ b/drivers/net/wireless/ath/ath9k/wmi.c
> @@ -334,7 +334,8 @@ int ath9k_wmi_cmd(struct wmi *wmi, enum wmi_cmd_id cmd_id,
>  			"Timeout waiting for WMI command: %s\n",
>  			wmi_cmd_to_name(cmd_id));
>  		mutex_unlock(&wmi->op_mutex);
> -		return -ETIMEDOUT;
> +		ret = -ETIMEDOUT;
> +		goto out;
>  	}
>  
>  	mutex_unlock(&wmi->op_mutex);
> -- 
> 1.7.0.4
> 
> 

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH] ath9k_htc: Fix few possible memory leaks

[Mohammed Shafi]

On Wed, Aug 24, 2011 at 11:18 PM, John W. Linville
<linville@tuxdriver.com> wrote:
> Is this intended for 3.1?  Should it be Cc: stable@kernel.org as well?

Hi John,

sorry for the inconvenience, please drop this patch
as discussed in the thread
http://comments.gmane.org/gmane.linux.kernel.wireless.general/75206
thanks!

>
> On Sun, Aug 21, 2011 at 09:24:45PM +0530, Mohammed Shafi Shajakhan wrote:
>> From: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>
>>
>> still there are few other memory leaks which will be fixed
>> very soon
>>
>> Cc: Rajkumar Manoharan <rmanohar@qca.qualcomm.com>
>> Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>
>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
>> ---
>>  drivers/net/wireless/ath/ath9k/htc_hst.c |   13 ++++++++-----
>>  drivers/net/wireless/ath/ath9k/wmi.c     |    3 ++-
>>  2 files changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/htc_hst.c b/drivers/net/wireless/ath/ath9k/htc_hst.c
>> index 1b90ed8..e435c9b 100644
>> --- a/drivers/net/wireless/ath/ath9k/htc_hst.c
>> +++ b/drivers/net/wireless/ath/ath9k/htc_hst.c
>> @@ -169,13 +169,14 @@ static int htc_config_pipe_credits(struct htc_target *target)
>>       time_left = wait_for_completion_timeout(&target->cmd_wait, HZ);
>>       if (!time_left) {
>>               dev_err(target->dev, "HTC credit config timeout\n");
>> -             return -ETIMEDOUT;
>> +             ret = -ETIMEDOUT;
>> +             goto err;
>>       }
>>
>>       return 0;
>>  err:
>>       kfree_skb(skb);
>> -     return -EINVAL;
>> +     return ret;
>>  }
>>
>>  static int htc_setup_complete(struct htc_target *target)
>> @@ -204,14 +205,15 @@ static int htc_setup_complete(struct htc_target *target)
>>       time_left = wait_for_completion_timeout(&target->cmd_wait, HZ);
>>       if (!time_left) {
>>               dev_err(target->dev, "HTC start timeout\n");
>> -             return -ETIMEDOUT;
>> +             ret = -ETIMEDOUT;
>> +             goto err;
>>       }
>>
>>       return 0;
>>
>>  err:
>>       kfree_skb(skb);
>> -     return -EINVAL;
>> +     return ret;
>>  }
>>
>>  /* HTC APIs */
>> @@ -276,7 +278,8 @@ int htc_connect_service(struct htc_target *target,
>>       if (!time_left) {
>>               dev_err(target->dev, "Service connection timeout for: %d\n",
>>                       service_connreq->service_id);
>> -             return -ETIMEDOUT;
>> +             ret = -ETIMEDOUT;
>> +             goto err;
>>       }
>>
>>       *conn_rsp_epid = target->conn_rsp_epid;
>> diff --git a/drivers/net/wireless/ath/ath9k/wmi.c b/drivers/net/wireless/ath/ath9k/wmi.c
>> index 35422fc..50d901d 100644
>> --- a/drivers/net/wireless/ath/ath9k/wmi.c
>> +++ b/drivers/net/wireless/ath/ath9k/wmi.c
>> @@ -334,7 +334,8 @@ int ath9k_wmi_cmd(struct wmi *wmi, enum wmi_cmd_id cmd_id,
>>                       "Timeout waiting for WMI command: %s\n",
>>                       wmi_cmd_to_name(cmd_id));
>>               mutex_unlock(&wmi->op_mutex);
>> -             return -ETIMEDOUT;
>> +             ret = -ETIMEDOUT;
>> +             goto out;
>>       }
>>
>>       mutex_unlock(&wmi->op_mutex);
>> --
>> 1.7.0.4
>>
>>
>
> --
> John W. Linville                Someday the world will need a hero, and you
> linville@tuxdriver.com                  might be all we have.  Be ready.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
shafi