Re: minor array overflow in ifconfig set80211chanlist()

2016-05-19 Thread Adrian Chadd
Ok, let's break out the chan array from the ieee chan value (8 bit).
We need that done for 11ac.




-adrian
___
freebsd-wireless@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-wireless
To unsubscribe, send any mail to "freebsd-wireless-unsubscr...@freebsd.org"


Re: minor array overflow in ifconfig set80211chanlist()

2016-05-19 Thread Andriy Voskoboinyk
Tue, 17 May 2016 01:05:57 +0300 було написано Andriy Voskoboinyk  
:


Tue, 17 May 2016 01:03:03 +0300 було написано Adrian Chadd  
:



Heh, god, it's used for both maximum ieee channel number /and/ the
array size? we should eventually fix that; 11ac channels will likely
overflow all of the above. :(


No (yes) :)
I mean ic->ic_nchans and nitems(ic->ic_channels)
... but you are right: ic_ieee is uint8_t, so it's limited by this  
number too.




... but there is another macro with the same value:
ieee80211_scan_sta.c:

#define MAX_IEEE_CHAN   256 /* max acceptable IEEE 
chan # */
CTASSERT(MAX_IEEE_CHAN >= 256);





-a
___
freebsd-wireless@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-wireless
To unsubscribe, send any mail to  
"freebsd-wireless-unsubscr...@freebsd.org"

___
freebsd-wireless@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-wireless
To unsubscribe, send any mail to  
"freebsd-wireless-unsubscr...@freebsd.org"

___
freebsd-wireless@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-wireless
To unsubscribe, send any mail to "freebsd-wireless-unsubscr...@freebsd.org"

Re: minor array overflow in ifconfig set80211chanlist()

2016-05-16 Thread Andriy Voskoboinyk
Tue, 17 May 2016 01:03:03 +0300 було написано Adrian Chadd  
:



Heh, god, it's used for both maximum ieee channel number /and/ the
array size? we should eventually fix that; 11ac channels will likely
overflow all of the above. :(


No (yes) :)
I mean ic->ic_nchans and nitems(ic->ic_channels)
... but you are right: ic_ieee is uint8_t, so it's limited by this number  
too.






-a
___
freebsd-wireless@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-wireless
To unsubscribe, send any mail to  
"freebsd-wireless-unsubscr...@freebsd.org"

___
freebsd-wireless@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-wireless
To unsubscribe, send any mail to "freebsd-wireless-unsubscr...@freebsd.org"

Re: minor array overflow in ifconfig set80211chanlist()

2016-05-16 Thread Adrian Chadd
On 16 May 2016 at 15:05, Andriy Voskoboinyk  wrote:
> Tue, 17 May 2016 01:03:03 +0300 було написано Adrian Chadd
> :
>
>> Heh, god, it's used for both maximum ieee channel number /and/ the
>> array size? we should eventually fix that; 11ac channels will likely
>> overflow all of the above. :(
>
>
> No (yes) :)
> I mean ic->ic_nchans and nitems(ic->ic_channels)
> ... but you are right: ic_ieee is uint8_t, so it's limited by this number
> too.

Right. Well, for 11ac they still have the IEEE number, but they also
have separate frequency definitions in the IEs for upper/lower 80MHz
and the configuration therein.



-adrian
___
freebsd-wireless@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-wireless
To unsubscribe, send any mail to "freebsd-wireless-unsubscr...@freebsd.org"

Re: minor array overflow in ifconfig set80211chanlist()

2016-05-16 Thread Adrian Chadd
Heh, god, it's used for both maximum ieee channel number /and/ the
array size? we should eventually fix that; 11ac channels will likely
overflow all of the above. :(



-a
___
freebsd-wireless@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-wireless
To unsubscribe, send any mail to "freebsd-wireless-unsubscr...@freebsd.org"


Re: minor array overflow in ifconfig set80211chanlist()

2016-05-16 Thread Andriy Voskoboinyk
Mon, 16 May 2016 22:42:50 +0300 було написано Don Lewis  
:



I asked adrian@ privately and he sent me here ...

Coverity is complaining about an array overflow in set80211chanlist().

The code in question is:
if (first > IEEE80211_CHAN_MAX)
errx(-1, "channel %u out of range, max  
%u",

first, IEEE80211_CHAN_MAX);
setbit(chanlist.ic_channels, first);

The value of IEEE80211_CHAN_MAX is 256, so first could be as large as
256 and setbit() would still be called.

The ifconfig man page says that channel numbers should be in the range
1 to 255, so I think the correct fix would be to change this test (as
well as others that follow) to >= IEEE80211_CHAN_MAX.

Does that look correct?


Yes, it's correct (however, there is no driver with such big channel table,
so it cannot be reproduced right now).
+ there is an overflow in the next (last > CHAN_MAX) check too.



Adrian suggested that maybe IEEE80211_CHAN_MAX should be 255.


It is already used as channel array size and max channel number;
changing it's meaning to [max array index] will require more changes
(one in regdomain_addchans(), more in net80211 and drivers).





___
freebsd-wireless@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-wireless
To unsubscribe, send any mail to  
"freebsd-wireless-unsubscr...@freebsd.org"

___
freebsd-wireless@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-wireless
To unsubscribe, send any mail to "freebsd-wireless-unsubscr...@freebsd.org"