[PATCH] staging: rtl8712: Free memory and return failure when kmalloc fails

[PATCH] staging: rtl8712: Free memory and return failure when kmalloc fails

[Souptick Joarder]

This patch is added to free memory and return failure when kmalloc fails

Signed-off-by: Souptick joarder <jrdr.linux@gmail.com>
---
 drivers/staging/rtl8712/os_intfs.c     | 3 ++-
 drivers/staging/rtl8712/rtl871x_cmd.c  | 5 ++++-
 drivers/staging/rtl8712/rtl871x_xmit.c | 5 ++++-
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8712/os_intfs.c b/drivers/staging/rtl8712/os_intfs.c
index cbe4de0..aab3141 100644
--- a/drivers/staging/rtl8712/os_intfs.c
+++ b/drivers/staging/rtl8712/os_intfs.c
@@ -313,7 +313,8 @@ u8 r8712_init_drv_sw(struct _adapter *padapter)
 		return _FAIL;
 	if (r8712_init_mlme_priv(padapter) == _FAIL)
 		return _FAIL;
-	_r8712_init_xmit_priv(&padapter->xmitpriv, padapter);
+	if ((_r8712_init_xmit_priv(&padapter->xmitpriv, padapter)) != _SUCCESS)
+		return _FAIL;
 	_r8712_init_recv_priv(&padapter->recvpriv, padapter);
 	memset((unsigned char *)&padapter->securitypriv, 0,
 	       sizeof(struct security_priv));
diff --git a/drivers/staging/rtl8712/rtl871x_cmd.c b/drivers/staging/rtl8712/rtl871x_cmd.c
index b7ee5e6..04638f1 100644
--- a/drivers/staging/rtl8712/rtl871x_cmd.c
+++ b/drivers/staging/rtl8712/rtl871x_cmd.c
@@ -72,8 +72,11 @@ static sint _init_cmd_priv(struct cmd_priv *pcmdpriv)
 			    ((addr_t)(pcmdpriv->cmd_allocated_buf) &
 			    (CMDBUFF_ALIGN_SZ - 1));
 	pcmdpriv->rsp_allocated_buf = kmalloc(MAX_RSPSZ + 4, GFP_ATOMIC);
-	if (!pcmdpriv->rsp_allocated_buf)
+	if (!pcmdpriv->rsp_allocated_buf) {
+		kfree(pcmdpriv->cmd_allocated_buf);
+		pcmdpriv->cmd_allocated_buf = NULL;
 		return _FAIL;
+	}
 	pcmdpriv->rsp_buf = pcmdpriv->rsp_allocated_buf  +  4 -
 			    ((addr_t)(pcmdpriv->rsp_allocated_buf) & 3);
 	pcmdpriv->cmd_issued_cnt = 0;
diff --git a/drivers/staging/rtl8712/rtl871x_xmit.c b/drivers/staging/rtl8712/rtl871x_xmit.c
index be38364..484d2f2 100644
--- a/drivers/staging/rtl8712/rtl871x_xmit.c
+++ b/drivers/staging/rtl8712/rtl871x_xmit.c
@@ -128,8 +128,11 @@ sint _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv,
 	_init_queue(&pxmitpriv->pending_xmitbuf_queue);
 	pxmitpriv->pallocated_xmitbuf = kmalloc(NR_XMITBUFF * sizeof(struct xmit_buf) + 4,
 						GFP_ATOMIC);
-	if (!pxmitpriv->pallocated_xmitbuf)
+	if (!pxmitpriv->pallocated_xmitbuf) {
+		kfree(pxmitpriv->pallocated_frame_buf);
+		pxmitpriv->pallocated_frame_buf = NULL;
 		return _FAIL;
+	}
 	pxmitpriv->pxmitbuf = pxmitpriv->pallocated_xmitbuf + 4 -
 			      ((addr_t)(pxmitpriv->pallocated_xmitbuf) & 3);
 	pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf;
-- 
1.9.1

Re: [PATCH] staging: rtl8712: Free memory and return failure when kmalloc fails

[Souptick Joarder]

Hi Larry, Greg,

On Thu, Oct 20, 2016 at 12:29 PM, Souptick Joarder <jrdr.linux@gmail.com> wrote:
> This patch is added to free memory and return failure when kmalloc fails
>
> Signed-off-by: Souptick joarder <jrdr.linux@gmail.com>
> ---
>  drivers/staging/rtl8712/os_intfs.c     | 3 ++-
>  drivers/staging/rtl8712/rtl871x_cmd.c  | 5 ++++-
>  drivers/staging/rtl8712/rtl871x_xmit.c | 5 ++++-
>  3 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/rtl8712/os_intfs.c b/drivers/staging/rtl8712/os_intfs.c
> index cbe4de0..aab3141 100644
> --- a/drivers/staging/rtl8712/os_intfs.c
> +++ b/drivers/staging/rtl8712/os_intfs.c
> @@ -313,7 +313,8 @@ u8 r8712_init_drv_sw(struct _adapter *padapter)
>                 return _FAIL;
>         if (r8712_init_mlme_priv(padapter) == _FAIL)
>                 return _FAIL;
> -       _r8712_init_xmit_priv(&padapter->xmitpriv, padapter);
> +       if ((_r8712_init_xmit_priv(&padapter->xmitpriv, padapter)) != _SUCCESS)
> +               return _FAIL;
>         _r8712_init_recv_priv(&padapter->recvpriv, padapter);
>         memset((unsigned char *)&padapter->securitypriv, 0,
>                sizeof(struct security_priv));
> diff --git a/drivers/staging/rtl8712/rtl871x_cmd.c b/drivers/staging/rtl8712/rtl871x_cmd.c
> index b7ee5e6..04638f1 100644
> --- a/drivers/staging/rtl8712/rtl871x_cmd.c
> +++ b/drivers/staging/rtl8712/rtl871x_cmd.c
> @@ -72,8 +72,11 @@ static sint _init_cmd_priv(struct cmd_priv *pcmdpriv)
>                             ((addr_t)(pcmdpriv->cmd_allocated_buf) &
>                             (CMDBUFF_ALIGN_SZ - 1));
>         pcmdpriv->rsp_allocated_buf = kmalloc(MAX_RSPSZ + 4, GFP_ATOMIC);
> -       if (!pcmdpriv->rsp_allocated_buf)
> +       if (!pcmdpriv->rsp_allocated_buf) {
> +               kfree(pcmdpriv->cmd_allocated_buf);
> +               pcmdpriv->cmd_allocated_buf = NULL;
>                 return _FAIL;
> +       }
>         pcmdpriv->rsp_buf = pcmdpriv->rsp_allocated_buf  +  4 -
>                             ((addr_t)(pcmdpriv->rsp_allocated_buf) & 3);
>         pcmdpriv->cmd_issued_cnt = 0;
> diff --git a/drivers/staging/rtl8712/rtl871x_xmit.c b/drivers/staging/rtl8712/rtl871x_xmit.c
> index be38364..484d2f2 100644
> --- a/drivers/staging/rtl8712/rtl871x_xmit.c
> +++ b/drivers/staging/rtl8712/rtl871x_xmit.c
> @@ -128,8 +128,11 @@ sint _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv,
>         _init_queue(&pxmitpriv->pending_xmitbuf_queue);
>         pxmitpriv->pallocated_xmitbuf = kmalloc(NR_XMITBUFF * sizeof(struct xmit_buf) + 4,
>                                                 GFP_ATOMIC);
> -       if (!pxmitpriv->pallocated_xmitbuf)
> +       if (!pxmitpriv->pallocated_xmitbuf) {
> +               kfree(pxmitpriv->pallocated_frame_buf);
> +               pxmitpriv->pallocated_frame_buf = NULL;
>                 return _FAIL;
> +       }
>         pxmitpriv->pxmitbuf = pxmitpriv->pallocated_xmitbuf + 4 -
>                               ((addr_t)(pxmitpriv->pallocated_xmitbuf) & 3);
>         pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf;
> --

Any Comment for this patch ?
> 1.9.1
>

Regards
Souptick

Re: [PATCH] staging: rtl8712: Free memory and return failure when kmalloc fails

[Greg KH]

On Mon, Oct 24, 2016 at 09:27:30AM +0530, Souptick Joarder wrote:
> Hi Larry, Greg,
> 
> On Thu, Oct 20, 2016 at 12:29 PM, Souptick Joarder <jrdr.linux@gmail.com> wrote:

<snip>

> Any Comment for this patch ?

It's only been a few days, for a staging driver patch, when loads of
other things were going on.  Please be patient, it will be reviewed
eventually.  Staging patches sometimes take up to 2 weeks to be reviewed
as they are at the bottom of my priority list, because, well, they are
staging patches :)

thanks,

greg k-h

Re: [PATCH] staging: rtl8712: Free memory and return failure when kmalloc fails

[Greg KH]

On Thu, Oct 20, 2016 at 12:29:33PM +0530, Souptick Joarder wrote:
> This patch is added to free memory and return failure when kmalloc fails

I'm sorry, but I can not parse that sentance.  Can you rephrase this a
bit better?  What exactly are you doing here?

> 
> Signed-off-by: Souptick joarder <jrdr.linux@gmail.com>
> ---
>  drivers/staging/rtl8712/os_intfs.c     | 3 ++-
>  drivers/staging/rtl8712/rtl871x_cmd.c  | 5 ++++-
>  drivers/staging/rtl8712/rtl871x_xmit.c | 5 ++++-
>  3 files changed, 10 insertions(+), 3 deletions(-)

Any reason why you didn't cc: the driverdevel mailing list?  I doubt
linux-wireless cares about staging drivers :(

> 
> diff --git a/drivers/staging/rtl8712/os_intfs.c b/drivers/staging/rtl8712/os_intfs.c
> index cbe4de0..aab3141 100644
> --- a/drivers/staging/rtl8712/os_intfs.c
> +++ b/drivers/staging/rtl8712/os_intfs.c
> @@ -313,7 +313,8 @@ u8 r8712_init_drv_sw(struct _adapter *padapter)
>  		return _FAIL;
>  	if (r8712_init_mlme_priv(padapter) == _FAIL)
>  		return _FAIL;
> -	_r8712_init_xmit_priv(&padapter->xmitpriv, padapter);
> +	if ((_r8712_init_xmit_priv(&padapter->xmitpriv, padapter)) != _SUCCESS)
> +		return _FAIL;

You don't have to unwind anything that r8712_init_mlme_priv() did?

>  	_r8712_init_recv_priv(&padapter->recvpriv, padapter);
>  	memset((unsigned char *)&padapter->securitypriv, 0,
>  	       sizeof(struct security_priv));

thanks,

greg k-h

Re: [PATCH] staging: rtl8712: Free memory and return failure when kmalloc fails

[Souptick Joarder]

Hi Greg,


On Tue, Oct 25, 2016 at 2:33 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Oct 20, 2016 at 12:29:33PM +0530, Souptick Joarder wrote:
>> This patch is added to free memory and return failure when kmalloc fails
>
> I'm sorry, but I can not parse that sentance.  Can you rephrase this a
> bit better?  What exactly are you doing here?

  There are few functions where we need to free previously allocated memory
  when kmalloc fails. Else it may lead to memory leakage.
  In  _init_cmd_priv() and _r8712_init_xmit_priv() , few places we are
not freeing
  previously allocated memory  when kmalloc fails.This patch will address it.

  shall I resend the patch?

>
>>
>> Signed-off-by: Souptick joarder <jrdr.linux@gmail.com>
>> ---
>>  drivers/staging/rtl8712/os_intfs.c     | 3 ++-
>>  drivers/staging/rtl8712/rtl871x_cmd.c  | 5 ++++-
>>  drivers/staging/rtl8712/rtl871x_xmit.c | 5 ++++-
>>  3 files changed, 10 insertions(+), 3 deletions(-)
>
> Any reason why you didn't cc: the driverdevel mailing list?  I doubt
> linux-wireless cares about staging drivers :(

    My Apology. I will add driverdevel mailing list in cc.

>
>>
>> diff --git a/drivers/staging/rtl8712/os_intfs.c b/drivers/staging/rtl8712/os_intfs.c
>> index cbe4de0..aab3141 100644
>> --- a/drivers/staging/rtl8712/os_intfs.c
>> +++ b/drivers/staging/rtl8712/os_intfs.c
>> @@ -313,7 +313,8 @@ u8 r8712_init_drv_sw(struct _adapter *padapter)
>>               return _FAIL;
>>       if (r8712_init_mlme_priv(padapter) == _FAIL)
>>               return _FAIL;
>> -     _r8712_init_xmit_priv(&padapter->xmitpriv, padapter);
>> +     if ((_r8712_init_xmit_priv(&padapter->xmitpriv, padapter)) != _SUCCESS)
>> +             return _FAIL;
>
> You don't have to unwind anything that r8712_init_mlme_priv() did?

  I didn't get your question?

  r8712_init_drv_sw() is getting called during initialization.
  if _r8712_init_xmit_priv() fails is it required to continue driver
initialization
  or return _FAIL similar like  previous function r8712_init_mlme_priv() ?


>
>>       _r8712_init_recv_priv(&padapter->recvpriv, padapter);
>>       memset((unsigned char *)&padapter->securitypriv, 0,
>>              sizeof(struct security_priv));
>
> thanks,
>
> greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rtl8712: Free memory and return failure when kmalloc fails

[Greg KH]

On Tue, Oct 25, 2016 at 10:06:48PM +0530, Souptick Joarder wrote:
> Hi Greg,
> 
> 
> On Tue, Oct 25, 2016 at 2:33 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Thu, Oct 20, 2016 at 12:29:33PM +0530, Souptick Joarder wrote:
> >> This patch is added to free memory and return failure when kmalloc fails
> >
> > I'm sorry, but I can not parse that sentance.  Can you rephrase this a
> > bit better?  What exactly are you doing here?
> 
>   There are few functions where we need to free previously allocated memory
>   when kmalloc fails. Else it may lead to memory leakage.
>   In  _init_cmd_priv() and _r8712_init_xmit_priv() , few places we are
> not freeing
>   previously allocated memory  when kmalloc fails.This patch will address it.
> 
>   shall I resend the patch?

Please do, it is long-gone from my queue, and put more text, like you
write here, in the changelog area.

> >> diff --git a/drivers/staging/rtl8712/os_intfs.c b/drivers/staging/rtl8712/os_intfs.c
> >> index cbe4de0..aab3141 100644
> >> --- a/drivers/staging/rtl8712/os_intfs.c
> >> +++ b/drivers/staging/rtl8712/os_intfs.c
> >> @@ -313,7 +313,8 @@ u8 r8712_init_drv_sw(struct _adapter *padapter)
> >>               return _FAIL;
> >>       if (r8712_init_mlme_priv(padapter) == _FAIL)
> >>               return _FAIL;
> >> -     _r8712_init_xmit_priv(&padapter->xmitpriv, padapter);
> >> +     if ((_r8712_init_xmit_priv(&padapter->xmitpriv, padapter)) != _SUCCESS)
> >> +             return _FAIL;
> >
> > You don't have to unwind anything that r8712_init_mlme_priv() did?
> 
>   I didn't get your question?
> 
>   r8712_init_drv_sw() is getting called during initialization.
>   if _r8712_init_xmit_priv() fails is it required to continue driver
> initialization
>   or return _FAIL similar like  previous function r8712_init_mlme_priv() ?

Are you sure that r8712_init_mlme_priv() does not allocate anything that
you need to now free?

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: rtl8712: Free memory and return failure when kmalloc fails

[Souptick Joarder]

On Tue, Oct 25, 2016 at 11:41 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Oct 25, 2016 at 10:06:48PM +0530, Souptick Joarder wrote:
>> Hi Greg,
>>
>>
>> On Tue, Oct 25, 2016 at 2:33 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > On Thu, Oct 20, 2016 at 12:29:33PM +0530, Souptick Joarder wrote:
>> >> This patch is added to free memory and return failure when kmalloc fails
>> >
>> > I'm sorry, but I can not parse that sentance.  Can you rephrase this a
>> > bit better?  What exactly are you doing here?
>>
>>   There are few functions where we need to free previously allocated memory
>>   when kmalloc fails. Else it may lead to memory leakage.
>>   In  _init_cmd_priv() and _r8712_init_xmit_priv() , few places we are
>> not freeing
>>   previously allocated memory  when kmalloc fails.This patch will address it.
>>
>>   shall I resend the patch?
>
> Please do, it is long-gone from my queue, and put more text, like you
> write here, in the changelog area.
>
>> >> diff --git a/drivers/staging/rtl8712/os_intfs.c b/drivers/staging/rtl8712/os_intfs.c
>> >> index cbe4de0..aab3141 100644
>> >> --- a/drivers/staging/rtl8712/os_intfs.c
>> >> +++ b/drivers/staging/rtl8712/os_intfs.c
>> >> @@ -313,7 +313,8 @@ u8 r8712_init_drv_sw(struct _adapter *padapter)
>> >>               return _FAIL;
>> >>       if (r8712_init_mlme_priv(padapter) == _FAIL)
>> >>               return _FAIL;
>> >> -     _r8712_init_xmit_priv(&padapter->xmitpriv, padapter);
>> >> +     if ((_r8712_init_xmit_priv(&padapter->xmitpriv, padapter)) != _SUCCESS)
>> >> +             return _FAIL;
>> >
>> > You don't have to unwind anything that r8712_init_mlme_priv() did?
>>
>>   I didn't get your question?
>>
>>   r8712_init_drv_sw() is getting called during initialization.
>>   if _r8712_init_xmit_priv() fails is it required to continue driver
>> initialization
>>   or return _FAIL similar like  previous function r8712_init_mlme_priv() ?
>
> Are you sure that r8712_init_mlme_priv() does not allocate anything that
> you need to now free?

  Yes you are right. We are not freeing few memories allocated in
r8712_init_mlme_priv()
  when _r8712_init_xmit_priv() return _FAIL. So we can't simply send
_FAIL without freeing
  those memories.

  I will resend the modified patch in a new mail.

>
> thanks,
>
> greg k-h