potential null deref in minstrel_ht_update_caps()?

potential null deref in minstrel_ht_update_caps()?

[Dan Carpenter]

This is a smatch thing.

net/mac80211/rc80211_minstrel_ht.c +639 minstrel_ht_update_caps(15)
	warn: variable dereferenced before check 'sta'
   631          struct ieee80211_mcs_info *mcs = &sta->ht_cap.mcs;
   632          struct ieee80211_local *local = hw_to_local(mp->hw);
   633          u16 sta_cap = sta->ht_cap.cap;
                              ^^^^^^^^^^^^^^^
	Dereferenced here.

   634          int ack_dur;
   635          int stbc;
   636          int i;
   637
   638          /* fall back to the old minstrel for legacy stations */
   639          if (sta && !sta->ht_cap.ht_supported) {
                    ^^^
	Checked here.

   640                  msp->is_ht = false;
   641                  memset(&msp->legacy, 0, sizeof(msp->legacy));

It seems like a bug, but I'm not sure how to deal with it.

regards,
dan carpenter

Re: potential null deref in minstrel_ht_update_caps()?

[John W. Linville]

On Thu, Jul 22, 2010 at 01:09:33PM +0200, Dan Carpenter wrote:
> This is a smatch thing.
> 
> net/mac80211/rc80211_minstrel_ht.c +639 minstrel_ht_update_caps(15)
> 	warn: variable dereferenced before check 'sta'
>    631          struct ieee80211_mcs_info *mcs = &sta->ht_cap.mcs;
>    632          struct ieee80211_local *local = hw_to_local(mp->hw);
>    633          u16 sta_cap = sta->ht_cap.cap;
>                               ^^^^^^^^^^^^^^^
> 	Dereferenced here.
> 
>    634          int ack_dur;
>    635          int stbc;
>    636          int i;
>    637
>    638          /* fall back to the old minstrel for legacy stations */
>    639          if (sta && !sta->ht_cap.ht_supported) {
>                     ^^^
> 	Checked here.
> 
>    640                  msp->is_ht = false;
>    641                  memset(&msp->legacy, 0, sizeof(msp->legacy));
> 
> It seems like a bug, but I'm not sure how to deal with it.

I think that sta NULL check is unnecessary there.

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

[PATCH] minstrel_ht: remove unnecessary NULL check in minstrel_ht_update_caps

[John W. Linville]

If sta is NULL, we will have problems long before we get here...

Reported-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
Cc: Felix Fietkau <nbd@openwrt.org>
---
 net/mac80211/rc80211_minstrel_ht.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index b5ace24..a16694b 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -636,7 +636,7 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
 	int i;
 
 	/* fall back to the old minstrel for legacy stations */
-	if (sta && !sta->ht_cap.ht_supported) {
+	if (!sta->ht_cap.ht_supported) {
 		msp->is_ht = false;
 		memset(&msp->legacy, 0, sizeof(msp->legacy));
 		msp->legacy.r = msp->ratelist;
-- 
1.7.1.1