* [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).