2014-12-14 22:36:33

by Rickard Strandqvist

[permalink] [raw]
Subject: [PATCH] staging: rtl8723au: os_dep: usb_intf.c: Fix for possible null pointer dereference

There is otherwise a risk of a possible null pointer dereference.

Was largely found by using a static code analysis program called cppcheck.

Signed-off-by: Rickard Strandqvist <[email protected]>
---
drivers/staging/rtl8723au/os_dep/usb_intf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723au/os_dep/usb_intf.c b/drivers/staging/rtl8723au/os_dep/usb_intf.c
index 865743e..71a6330 100644
--- a/drivers/staging/rtl8723au/os_dep/usb_intf.c
+++ b/drivers/staging/rtl8723au/os_dep/usb_intf.c
@@ -351,10 +351,11 @@ error_exit:
int rtw_hw_resume23a(struct rtw_adapter *padapter)
{
struct pwrctrl_priv *pwrpriv = &padapter->pwrctrlpriv;
- struct net_device *pnetdev = padapter->pnetdev;
+ struct net_device *pnetdev;

if (padapter) { /* system resume */
DBG_8723A("==> rtw_hw_resume23a\n");
+ pnetdev = padapter->pnetdev;
down(&pwrpriv->lock);
pwrpriv->bips_processing = true;
rtw_reset_drv_sw23a(padapter);
--
1.7.10.4



2014-12-16 00:04:22

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8723au: os_dep: usb_intf.c: Fix for possible null pointer dereference

On 12/15/2014 05:01 PM, Rickard Strandqvist wrote:
> Hi
>
> No the rtw_hw_resume23a() is not used anywhere.
> I also do a check of all functions that are not used, but not in the
> drivers/staging, I suspected that these might be under development and
> used in the future.
>
> What do you want to do, who decides?

The original developers of this code do not include any of the Linux
maintainers. If you see an unused routine, submit a patch to delete it. If a
maintainer thinks it should be kept, your patch will be NACKed, but that is pnot
likely.

Larry



2014-12-15 10:31:30

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8723au: os_dep: usb_intf.c: Fix for possible null pointer dereference

On Sun, Dec 14, 2014 at 11:39:14PM +0100, Rickard Strandqvist wrote:
> There is otherwise a risk of a possible null pointer dereference.
>
> Was largely found by using a static code analysis program called cppcheck.
>
> Signed-off-by: Rickard Strandqvist <[email protected]>
> ---
> drivers/staging/rtl8723au/os_dep/usb_intf.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8723au/os_dep/usb_intf.c b/drivers/staging/rtl8723au/os_dep/usb_intf.c
> index 865743e..71a6330 100644
> --- a/drivers/staging/rtl8723au/os_dep/usb_intf.c
> +++ b/drivers/staging/rtl8723au/os_dep/usb_intf.c
> @@ -351,10 +351,11 @@ error_exit:
> int rtw_hw_resume23a(struct rtw_adapter *padapter)

That's weird. Is this function even called?

regards,
dan carpenter


2014-12-15 23:01:42

by Rickard Strandqvist

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8723au: os_dep: usb_intf.c: Fix for possible null pointer dereference

Hi

No the rtw_hw_resume23a() is not used anywhere.
I also do a check of all functions that are not used, but not in the
drivers/staging, I suspected that these might be under development and
used in the future.

What do you want to do, who decides?


Kind regards
Rickard Strandqvist


2014-12-15 16:48 GMT+01:00 Jes Sorensen <[email protected]>:
> Dan Carpenter <[email protected]> writes:
>> On Sun, Dec 14, 2014 at 11:39:14PM +0100, Rickard Strandqvist wrote:
>>> There is otherwise a risk of a possible null pointer dereference.
>>>
>>> Was largely found by using a static code analysis program called cppcheck.
>>>
>>> Signed-off-by: Rickard Strandqvist <[email protected]>
>>> ---
>>> drivers/staging/rtl8723au/os_dep/usb_intf.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/rtl8723au/os_dep/usb_intf.c
>>> b/drivers/staging/rtl8723au/os_dep/usb_intf.c
>>> index 865743e..71a6330 100644
>>> --- a/drivers/staging/rtl8723au/os_dep/usb_intf.c
>>> +++ b/drivers/staging/rtl8723au/os_dep/usb_intf.c
>>> @@ -351,10 +351,11 @@ error_exit:
>>> int rtw_hw_resume23a(struct rtw_adapter *padapter)
>>
>> That's weird. Is this function even called?
>
> [jes@ultrasam jes.git]$ find drivers/staging/rtl8723au -name \*.[ch] |xargs grep rtw_hw_resume
> drivers/staging/rtl8723au/include/osdep_intf.h:int rtw_hw_resume23a(struct rtw_adapter *padapter);
> drivers/staging/rtl8723au/os_dep/usb_intf.c:int rtw_hw_resume23a(struct rtw_adapter *padapter)
> drivers/staging/rtl8723au/os_dep/usb_intf.c: DBG_8723A("==> rtw_hw_resume23a\n");
> [jes@ultrasam jes.git]$ find drivers/staging/rtl8723au -name \*.[ch] |xargs grep rtw_hw_suspend
> drivers/staging/rtl8723au/include/osdep_intf.h:int rtw_hw_suspend23a(struct rtw_adapter *padapter);
> drivers/staging/rtl8723au/os_dep/usb_intf.c:int rtw_hw_suspend23a(struct rtw_adapter *padapter)
> drivers/staging/rtl8723au/os_dep/usb_intf.c: DBG_8723A("==> rtw_hw_suspend23a\n");
>
> A more useful patch would be one removing those two functions IMHO.
>
> Jes

2014-12-15 15:48:56

by Jes Sorensen

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8723au: os_dep: usb_intf.c: Fix for possible null pointer dereference

Dan Carpenter <[email protected]> writes:
> On Sun, Dec 14, 2014 at 11:39:14PM +0100, Rickard Strandqvist wrote:
>> There is otherwise a risk of a possible null pointer dereference.
>>
>> Was largely found by using a static code analysis program called cppcheck.
>>
>> Signed-off-by: Rickard Strandqvist <[email protected]>
>> ---
>> drivers/staging/rtl8723au/os_dep/usb_intf.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/rtl8723au/os_dep/usb_intf.c
>> b/drivers/staging/rtl8723au/os_dep/usb_intf.c
>> index 865743e..71a6330 100644
>> --- a/drivers/staging/rtl8723au/os_dep/usb_intf.c
>> +++ b/drivers/staging/rtl8723au/os_dep/usb_intf.c
>> @@ -351,10 +351,11 @@ error_exit:
>> int rtw_hw_resume23a(struct rtw_adapter *padapter)
>
> That's weird. Is this function even called?

[jes@ultrasam jes.git]$ find drivers/staging/rtl8723au -name \*.[ch] |xargs grep rtw_hw_resume
drivers/staging/rtl8723au/include/osdep_intf.h:int rtw_hw_resume23a(struct rtw_adapter *padapter);
drivers/staging/rtl8723au/os_dep/usb_intf.c:int rtw_hw_resume23a(struct rtw_adapter *padapter)
drivers/staging/rtl8723au/os_dep/usb_intf.c: DBG_8723A("==> rtw_hw_resume23a\n");
[jes@ultrasam jes.git]$ find drivers/staging/rtl8723au -name \*.[ch] |xargs grep rtw_hw_suspend
drivers/staging/rtl8723au/include/osdep_intf.h:int rtw_hw_suspend23a(struct rtw_adapter *padapter);
drivers/staging/rtl8723au/os_dep/usb_intf.c:int rtw_hw_suspend23a(struct rtw_adapter *padapter)
drivers/staging/rtl8723au/os_dep/usb_intf.c: DBG_8723A("==> rtw_hw_suspend23a\n");

A more useful patch would be one removing those two functions IMHO.

Jes

2014-12-17 20:07:08

by Rickard Strandqvist

[permalink] [raw]
Subject: Re: [PATCH] staging: rtl8723au: os_dep: usb_intf.c: Fix for possible null pointer dereference

Hi

Ok, do a patch to remove it now.

Kind regards
Rickard Strandqvist


2014-12-16 1:04 GMT+01:00 Larry Finger <[email protected]>:
> On 12/15/2014 05:01 PM, Rickard Strandqvist wrote:
>>
>> Hi
>>
>> No the rtw_hw_resume23a() is not used anywhere.
>> I also do a check of all functions that are not used, but not in the
>> drivers/staging, I suspected that these might be under development and
>> used in the future.
>>
>> What do you want to do, who decides?
>
>
> The original developers of this code do not include any of the Linux
> maintainers. If you see an unused routine, submit a patch to delete it. If a
> maintainer thinks it should be kept, your patch will be NACKed, but that is
> pnot likely.
>
> Larry
>
>