2012-07-03 10:53:52

by Dan Carpenter

[permalink] [raw]
Subject: re: cfg80211: add 802.11ad (60gHz band) support

Hello Vladimir Kondratiev,

The patch 3a0c52a6d82c: "cfg80211: add 802.11ad (60gHz band) support"
from Jul 2, 2012, leads to the following warning:
drivers/net/wireless/mac80211_hwsim.c:1841 init_mac80211_hwsim()
warn: buffer overflow 'data->bands' 2 <= 2

drivers/net/wireless/mac80211_hwsim.c
1839
1840 for (band = IEEE80211_BAND_2GHZ; band < IEEE80211_NUM_BANDS; band++) {
^^^^^^^^^^^^^^^^^^^
We raised IEEE80211_NUM_BANDS to 3.

1841 struct ieee80211_supported_band *sband = &data->bands[band];
^^^^^^^^^^^
This only has 2 elements still.

1842 switch (band) {
1843 case IEEE80211_BAND_2GHZ:

It causes a bogus dereference later.

regards,
dan carpenter



2012-07-03 11:42:33

by Johannes Berg

[permalink] [raw]
Subject: Re: cfg80211: add 802.11ad (60gHz band) support

On Tue, 2012-07-03 at 14:32 +0300, Dan Carpenter wrote:

> > Thanks Dan!
> >
> > Is this one of your non-default tests? I think I ran smatch on this, but
> > it might be old.
>
> It should be on by default in the latest code. :)

I'll update anyway, but because of the huge number of warnings in all
the various wireless drives I didn't actually run it, I only ran it on
the mac80211 & cfg80211 code ... oops.

johannes


2012-07-03 11:13:56

by Vladimir Kondratiev

[permalink] [raw]
Subject: Re: cfg80211: add 802.11ad (60gHz band) support

On Tuesday, July 03, 2012 01:04:17 PM Johannes Berg wrote:
> Thanks Dan!
>
> Is this one of your non-default tests? I think I ran smatch on this, but
> it might be old. Anyway, I'll commit this fix:
>
> http://p.sipsolutions.net/a1c40eea7e33541b.txt
>
> johannes

Yes, exactly!

Was about to mail same patch.

What is the best method to catch such cases?
"make allmodconfig", or is there something better?

Tnanks, Vladimir

2012-07-03 11:35:05

by Dan Carpenter

[permalink] [raw]
Subject: Re: cfg80211: add 802.11ad (60gHz band) support

On Tue, Jul 03, 2012 at 02:13:49PM +0300, Vladimir Kondratiev wrote:
> On Tuesday, July 03, 2012 01:04:17 PM Johannes Berg wrote:
> > Thanks Dan!
> >
> > Is this one of your non-default tests? I think I ran smatch on this, but
> > it might be old. Anyway, I'll commit this fix:
> >
> > http://p.sipsolutions.net/a1c40eea7e33541b.txt
> >
> > johannes
>
> Yes, exactly!
>
> Was about to mail same patch.
>
> What is the best method to catch such cases?
> "make allmodconfig", or is there something better?

This was a Smatch warning.

http://smatch.sourceforge.net/

regards,
dan carpenter


2012-07-03 11:04:24

by Johannes Berg

[permalink] [raw]
Subject: Re: cfg80211: add 802.11ad (60gHz band) support

On Tue, 2012-07-03 at 13:53 +0300, Dan Carpenter wrote:
> Hello Vladimir Kondratiev,
>
> The patch 3a0c52a6d82c: "cfg80211: add 802.11ad (60gHz band) support"
> from Jul 2, 2012, leads to the following warning:
> drivers/net/wireless/mac80211_hwsim.c:1841 init_mac80211_hwsim()
> warn: buffer overflow 'data->bands' 2 <= 2
>
> drivers/net/wireless/mac80211_hwsim.c
> 1839
> 1840 for (band = IEEE80211_BAND_2GHZ; band < IEEE80211_NUM_BANDS; band++) {
> ^^^^^^^^^^^^^^^^^^^
> We raised IEEE80211_NUM_BANDS to 3.
>
> 1841 struct ieee80211_supported_band *sband = &data->bands[band];
> ^^^^^^^^^^^
> This only has 2 elements still.
>
> 1842 switch (band) {
> 1843 case IEEE80211_BAND_2GHZ:
>
> It causes a bogus dereference later.

Thanks Dan!

Is this one of your non-default tests? I think I ran smatch on this, but
it might be old. Anyway, I'll commit this fix:

http://p.sipsolutions.net/a1c40eea7e33541b.txt

johannes


2012-07-03 11:32:36

by Dan Carpenter

[permalink] [raw]
Subject: Re: cfg80211: add 802.11ad (60gHz band) support

On Tue, Jul 03, 2012 at 01:04:17PM +0200, Johannes Berg wrote:
> On Tue, 2012-07-03 at 13:53 +0300, Dan Carpenter wrote:
> > Hello Vladimir Kondratiev,
> >
> > The patch 3a0c52a6d82c: "cfg80211: add 802.11ad (60gHz band) support"
> > from Jul 2, 2012, leads to the following warning:
> > drivers/net/wireless/mac80211_hwsim.c:1841 init_mac80211_hwsim()
> > warn: buffer overflow 'data->bands' 2 <= 2
> >
> > drivers/net/wireless/mac80211_hwsim.c
> > 1839
> > 1840 for (band = IEEE80211_BAND_2GHZ; band < IEEE80211_NUM_BANDS; band++) {
> > ^^^^^^^^^^^^^^^^^^^
> > We raised IEEE80211_NUM_BANDS to 3.
> >
> > 1841 struct ieee80211_supported_band *sband = &data->bands[band];
> > ^^^^^^^^^^^
> > This only has 2 elements still.
> >
> > 1842 switch (band) {
> > 1843 case IEEE80211_BAND_2GHZ:
> >
> > It causes a bogus dereference later.
>
> Thanks Dan!
>
> Is this one of your non-default tests? I think I ran smatch on this, but
> it might be old.

It should be on by default in the latest code. :)

regards,
dan carpenter