diff mbox

staging: rtl8723au: os_dep: usb_intf.c: Fix for possible null pointer dereference

Message ID 1418596754-24735-1-git-send-email-rickard_strandqvist@spectrumdigital.se (mailing list archive)
State Not Applicable
Headers show

Commit Message

Rickard Strandqvist Dec. 14, 2014, 10:39 p.m. UTC
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 <rickard_strandqvist@spectrumdigital.se>
---
 drivers/staging/rtl8723au/os_dep/usb_intf.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Dan Carpenter Dec. 15, 2014, 10:31 a.m. UTC | #1
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 <rickard_strandqvist@spectrumdigital.se>
> ---
>  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

--
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
Jes Sorensen Dec. 15, 2014, 3:48 p.m. UTC | #2
Dan Carpenter <dan.carpenter@oracle.com> 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 <rickard_strandqvist@spectrumdigital.se>
>> ---
>>  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
--
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
Rickard Strandqvist Dec. 15, 2014, 11:01 p.m. UTC | #3
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 <Jes.Sorensen@redhat.com>:
> Dan Carpenter <dan.carpenter@oracle.com> 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 <rickard_strandqvist@spectrumdigital.se>
>>> ---
>>>  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
--
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
Larry Finger Dec. 16, 2014, 12:04 a.m. UTC | #4
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


--
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
Rickard Strandqvist Dec. 17, 2014, 8:06 p.m. UTC | #5
Hi

Ok, do a patch to remove it now.

Kind regards
Rickard Strandqvist


2014-12-16 1:04 GMT+01:00 Larry Finger <Larry.Finger@lwfinger.net>:
> 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
>
>
--
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
diff mbox

Patch

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);