* [PATCH] fix error handling in drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
@ 2018-09-26 17:52 valdis.kletnieks
2018-09-26 18:04 ` Kees Cook
2018-09-27 14:24 ` Dan Carpenter
0 siblings, 2 replies; 4+ messages in thread
From: valdis.kletnieks @ 2018-09-26 17:52 UTC (permalink / raw)
To: gregkh, John Whitmore, Kees Cook; +Cc: kernelnewbies, linux-kernel, devel
John notes that if the kzalloc of ieee->pHTInfo fails, we fail to call
ieee80211_networks_free(). In addition, that function has an un-needed check
before kfree().
Reported-by: John Whitmore <arigead@gmail.com>
Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
---
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
index 90a097f2cd4e..97ff0371b5bb 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
@@ -78,8 +78,6 @@ static inline int ieee80211_networks_allocate(struct ieee80211_device *ieee)
static inline void ieee80211_networks_free(struct ieee80211_device *ieee)
{
- if (!ieee->networks)
- return;
kfree(ieee->networks);
ieee->networks = NULL;
}
@@ -180,6 +178,7 @@ struct net_device *alloc_ieee80211(int sizeof_priv)
return dev;
failed:
+ ieee80211_networks_free(ieee);
if (dev)
free_netdev(dev);
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] fix error handling in drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
2018-09-26 17:52 [PATCH] fix error handling in drivers/staging/rtl8192u/ieee80211/ieee80211_module.c valdis.kletnieks
@ 2018-09-26 18:04 ` Kees Cook
2018-09-27 14:24 ` Dan Carpenter
1 sibling, 0 replies; 4+ messages in thread
From: Kees Cook @ 2018-09-26 18:04 UTC (permalink / raw)
To: Valdis Kletnieks
Cc: Greg KH, John Whitmore, kernelnewbies, LKML, open list:ANDROID DRIVERS
On Wed, Sep 26, 2018 at 10:52 AM, <valdis.kletnieks@vt.edu> wrote:
> John notes that if the kzalloc of ieee->pHTInfo fails, we fail to call
> ieee80211_networks_free(). In addition, that function has an un-needed check
> before kfree().
>
> Reported-by: John Whitmore <arigead@gmail.com>
> Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
Reviewed-by: Kees Cook <keescook@chromium.org>
(And this seems to be the only case of this -- I don't see this code
trivially copy/pasted in other 80211 stacks.)
-Kees
> ---
> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
> index 90a097f2cd4e..97ff0371b5bb 100644
> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
> @@ -78,8 +78,6 @@ static inline int ieee80211_networks_allocate(struct ieee80211_device *ieee)
>
> static inline void ieee80211_networks_free(struct ieee80211_device *ieee)
> {
> - if (!ieee->networks)
> - return;
> kfree(ieee->networks);
> ieee->networks = NULL;
> }
> @@ -180,6 +178,7 @@ struct net_device *alloc_ieee80211(int sizeof_priv)
> return dev;
>
> failed:
> + ieee80211_networks_free(ieee);
> if (dev)
> free_netdev(dev);
>
>
--
Kees Cook
Pixel Security
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fix error handling in drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
2018-09-26 17:52 [PATCH] fix error handling in drivers/staging/rtl8192u/ieee80211/ieee80211_module.c valdis.kletnieks
2018-09-26 18:04 ` Kees Cook
@ 2018-09-27 14:24 ` Dan Carpenter
2018-09-27 15:43 ` Kees Cook
1 sibling, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2018-09-27 14:24 UTC (permalink / raw)
To: valdis.kletnieks
Cc: gregkh, John Whitmore, Kees Cook, devel, linux-kernel, kernelnewbies
On Wed, Sep 26, 2018 at 01:52:17PM -0400, valdis.kletnieks@vt.edu wrote:
> John notes that if the kzalloc of ieee->pHTInfo fails, we fail to call
> ieee80211_networks_free(). In addition, that function has an un-needed check
> before kfree().
>
> Reported-by: John Whitmore <arigead@gmail.com>
> Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
> ---
> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
> index 90a097f2cd4e..97ff0371b5bb 100644
> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
> @@ -78,8 +78,6 @@ static inline int ieee80211_networks_allocate(struct ieee80211_device *ieee)
>
> static inline void ieee80211_networks_free(struct ieee80211_device *ieee)
> {
> - if (!ieee->networks)
> - return;
> kfree(ieee->networks);
> ieee->networks = NULL;
> }
> @@ -180,6 +178,7 @@ struct net_device *alloc_ieee80211(int sizeof_priv)
> return dev;
>
> failed:
> + ieee80211_networks_free(ieee);
> if (dev)
> free_netdev(dev);
When there is a "goto failed;" then it's called "one err style" error
handling and we're just asking for bugs... In this case the bug is that
we're not allowd to call ieee80211_networks_free() with a NULL
"ieee" parameter. The right thing to do is to only call
ieee80211_networks_free() if we know that ieee80211_networks_allocate()
succeeded.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fix error handling in drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
2018-09-27 14:24 ` Dan Carpenter
@ 2018-09-27 15:43 ` Kees Cook
0 siblings, 0 replies; 4+ messages in thread
From: Kees Cook @ 2018-09-27 15:43 UTC (permalink / raw)
To: Dan Carpenter
Cc: Valdis Kletnieks, Greg KH, John Whitmore,
open list:ANDROID DRIVERS, LKML, kernelnewbies
On Thu, Sep 27, 2018 at 7:24 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Wed, Sep 26, 2018 at 01:52:17PM -0400, valdis.kletnieks@vt.edu wrote:
>> John notes that if the kzalloc of ieee->pHTInfo fails, we fail to call
>> ieee80211_networks_free(). In addition, that function has an un-needed check
>> before kfree().
>>
>> Reported-by: John Whitmore <arigead@gmail.com>
>> Signed-off-by: Valdis Kletnieks <valdis.kletnieks@vt.edu>
>> ---
>> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
>> index 90a097f2cd4e..97ff0371b5bb 100644
>> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
>> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
>> @@ -78,8 +78,6 @@ static inline int ieee80211_networks_allocate(struct ieee80211_device *ieee)
>>
>> static inline void ieee80211_networks_free(struct ieee80211_device *ieee)
>> {
>> - if (!ieee->networks)
>> - return;
>> kfree(ieee->networks);
>> ieee->networks = NULL;
>> }
>> @@ -180,6 +178,7 @@ struct net_device *alloc_ieee80211(int sizeof_priv)
>> return dev;
>>
>> failed:
>> + ieee80211_networks_free(ieee);
>> if (dev)
>> free_netdev(dev);
>
> When there is a "goto failed;" then it's called "one err style" error
> handling and we're just asking for bugs... In this case the bug is that
> we're not allowd to call ieee80211_networks_free() with a NULL
> "ieee" parameter. The right thing to do is to only call
> ieee80211_networks_free() if we know that ieee80211_networks_allocate()
> succeeded.
Ah yeah, thanks for the extra eyes. Looks like it would need something
more like this:
diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
index 90a097f2cd4e..8d6a28af96dc 100644
--- a/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
+++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_module.c
@@ -78,7 +78,7 @@ static inline int ieee80211_networks_allocate(struct
ieee80211_device *ieee)
static inline void ieee80211_networks_free(struct ieee80211_device *ieee)
{
- if (!ieee->networks)
+ if (!ieee)
return;
kfree(ieee->networks);
ieee->networks = NULL;
@@ -97,7 +97,7 @@ static inline void
ieee80211_networks_initialize(struct ieee80211_device *ieee)
struct net_device *alloc_ieee80211(int sizeof_priv)
{
- struct ieee80211_device *ieee;
+ struct ieee80211_device *ieee = NULL;
struct net_device *dev;
int i, err;
@@ -180,6 +180,7 @@ struct net_device *alloc_ieee80211(int sizeof_priv)
return dev;
failed:
+ ieee80211_networks_free(ieee);
if (dev)
free_netdev(dev);
Valdis, can you respin the patch?
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-09-27 15:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-26 17:52 [PATCH] fix error handling in drivers/staging/rtl8192u/ieee80211/ieee80211_module.c valdis.kletnieks
2018-09-26 18:04 ` Kees Cook
2018-09-27 14:24 ` Dan Carpenter
2018-09-27 15:43 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).