minstrel doesn't count max rate count in fact, since it doesn't use
a loop variable `i' and hence allocs space only for bitrates found in
the first band.
Fix it by involving the `i' as an index so that it traverses all the
bands now and finds the real max bitrate count.
Signed-off-by: Jiri Slaby <[email protected]>
Cc: Felix Fietkau <[email protected]>
---
net/mac80211/rc80211_minstrel.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/mac80211/rc80211_minstrel.c b/net/mac80211/rc80211_minstrel.c
index 3824990..70df3dc 100644
--- a/net/mac80211/rc80211_minstrel.c
+++ b/net/mac80211/rc80211_minstrel.c
@@ -476,7 +476,7 @@ minstrel_alloc_sta(void *priv, struct ieee80211_sta *sta, gfp_t gfp)
return NULL;
for (i = 0; i < IEEE80211_NUM_BANDS; i++) {
- sband = hw->wiphy->bands[hw->conf.channel->band];
+ sband = hw->wiphy->bands[i];
if (sband->n_bitrates > max_rates)
max_rates = sband->n_bitrates;
}
--
1.6.2.4
On Fri, 15 May 2009, John W. Linville wrote:
>
> Unfortunately, I think Dave might have limited availability right now...
Yeah, I think he's on some Alaskan cruise.
Should I just pull directly from you this time?
Linus
On 05/15/2009 08:21 PM, Heinz Diehl wrote:
> This patchset crashes my WLAN. Reverting it does fix this:
Could you try this fix
http://git.kernel.org/?p=linux/kernel/git/linville/wireless-testing.git;a=commitdiff_plain;h=b3f25c62aa41e0c372536c0f3975b39f21217b57;hp=30e2341dd2c82780eddd785eb54695115c6aea8d
?
Hi Linus,
> > Unfortunately, I think Dave might have limited availability right now...
>
> Yeah, I think he's on some Alaskan cruise.
>
> Should I just pull directly from you this time?
I would have a similar pull request pending for Bluetooth fixes:
http://marc.info/?l=linux-netdev&m=124209765506433
Regards
Marcel
On Mon, May 4, 2009 at 9:04 AM, Jiri Slaby <[email protected]> wrote:
> minstrel doesn't count max rate count in fact, since it doesn't use
> a loop variable `i' and hence allocs space only for bitrates found in
> the first band.
>
> Fix it by involving the `i' as an index so that it traverses all the
> bands now and finds the real max bitrate count.
>
> Signed-off-by: Jiri Slaby <[email protected]>
> Cc: Felix Fietkau <[email protected]>
Cc: [email protected]
Luis
pid doesn't count with some band having more bitrates than the one
associated the first time.
Fix that by counting the maximal available bitrate count and allocate
big enough space.
Secondly, fix touching uninitialized memory which causes panics.
Index sucked from this random memory points to the hell.
The fix is to sort the rates on each band change.
Signed-off-by: Jiri Slaby <[email protected]>
---
net/mac80211/rc80211_pid_algo.c | 71 +++++++++++++++++++++-----------------
1 files changed, 39 insertions(+), 32 deletions(-)
diff --git a/net/mac80211/rc80211_pid_algo.c b/net/mac80211/rc80211_pid_algo.c
index b16801c..c318687 100644
--- a/net/mac80211/rc80211_pid_algo.c
+++ b/net/mac80211/rc80211_pid_algo.c
@@ -317,13 +317,44 @@ rate_control_pid_rate_init(void *priv, struct ieee80211_supported_band *sband,
struct ieee80211_sta *sta, void *priv_sta)
{
struct rc_pid_sta_info *spinfo = priv_sta;
+ struct rc_pid_info *pinfo = priv;
+ struct rc_pid_rateinfo *rinfo = pinfo->rinfo;
struct sta_info *si;
+ int i, j, tmp;
+ bool s;
/* TODO: This routine should consider using RSSI from previous packets
* as we need to have IEEE 802.1X auth succeed immediately after assoc..
* Until that method is implemented, we will use the lowest supported
* rate as a workaround. */
+ /* Sort the rates. This is optimized for the most common case (i.e.
+ * almost-sorted CCK+OFDM rates). Kind of bubble-sort with reversed
+ * mapping too. */
+ for (i = 0; i < sband->n_bitrates; i++) {
+ rinfo[i].index = i;
+ rinfo[i].rev_index = i;
+ if (RC_PID_FAST_START)
+ rinfo[i].diff = 0;
+ else
+ rinfo[i].diff = i * pinfo->norm_offset;
+ }
+ for (i = 1; i < sband->n_bitrates; i++) {
+ s = 0;
+ for (j = 0; j < sband->n_bitrates - i; j++)
+ if (unlikely(sband->bitrates[rinfo[j].index].bitrate >
+ sband->bitrates[rinfo[j + 1].index].bitrate)) {
+ tmp = rinfo[j].index;
+ rinfo[j].index = rinfo[j + 1].index;
+ rinfo[j + 1].index = tmp;
+ rinfo[rinfo[j].index].rev_index = j;
+ rinfo[rinfo[j + 1].index].rev_index = j + 1;
+ s = 1;
+ }
+ if (!s)
+ break;
+ }
+
spinfo->txrate_idx = rate_lowest_index(sband, sta);
/* HACK */
si = container_of(sta, struct sta_info, sta);
@@ -336,21 +367,24 @@ static void *rate_control_pid_alloc(struct ieee80211_hw *hw,
struct rc_pid_info *pinfo;
struct rc_pid_rateinfo *rinfo;
struct ieee80211_supported_band *sband;
- int i, j, tmp;
- bool s;
+ int i, max_rates = 0;
#ifdef CONFIG_MAC80211_DEBUGFS
struct rc_pid_debugfs_entries *de;
#endif
- sband = hw->wiphy->bands[hw->conf.channel->band];
-
pinfo = kmalloc(sizeof(*pinfo), GFP_ATOMIC);
if (!pinfo)
return NULL;
+ for (i = 0; i < IEEE80211_NUM_BANDS; i++) {
+ sband = hw->wiphy->bands[i];
+ if (sband->n_bitrates > max_rates)
+ max_rates = sband->n_bitrates;
+ }
+
/* We can safely assume that sband won't change unless we get
* reinitialized. */
- rinfo = kmalloc(sizeof(*rinfo) * sband->n_bitrates, GFP_ATOMIC);
+ rinfo = kmalloc(sizeof(*rinfo) * max_rates, GFP_ATOMIC);
if (!rinfo) {
kfree(pinfo);
return NULL;
@@ -368,33 +402,6 @@ static void *rate_control_pid_alloc(struct ieee80211_hw *hw,
pinfo->rinfo = rinfo;
pinfo->oldrate = 0;
- /* Sort the rates. This is optimized for the most common case (i.e.
- * almost-sorted CCK+OFDM rates). Kind of bubble-sort with reversed
- * mapping too. */
- for (i = 0; i < sband->n_bitrates; i++) {
- rinfo[i].index = i;
- rinfo[i].rev_index = i;
- if (RC_PID_FAST_START)
- rinfo[i].diff = 0;
- else
- rinfo[i].diff = i * pinfo->norm_offset;
- }
- for (i = 1; i < sband->n_bitrates; i++) {
- s = 0;
- for (j = 0; j < sband->n_bitrates - i; j++)
- if (unlikely(sband->bitrates[rinfo[j].index].bitrate >
- sband->bitrates[rinfo[j + 1].index].bitrate)) {
- tmp = rinfo[j].index;
- rinfo[j].index = rinfo[j + 1].index;
- rinfo[j + 1].index = tmp;
- rinfo[rinfo[j].index].rev_index = j;
- rinfo[rinfo[j + 1].index].rev_index = j + 1;
- s = 1;
- }
- if (!s)
- break;
- }
-
#ifdef CONFIG_MAC80211_DEBUGFS
de = &pinfo->dentries;
de->target = debugfs_create_u32("target_pf", S_IRUSR | S_IWUSR,
--
1.6.2.4
On 04.05.2009, Jiri Slaby wrote:
> minstrel doesn't count max rate count in fact, since it doesn't use
> a loop variable `i' and hence allocs space only for bitrates found in
> the first band.
[....]
This patchset crashes my WLAN. Reverting it does fix this:
[....]
wlan0: associated
BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
IP: [<ffffffffa0273b0f>] minstrel_alloc_sta+0x6f/0xf0 [mac80211]
PGD 229da2067 PUD 229d07067 PMD 0
Oops: 0000 [#1] PREEMPT SMP
last sysfs file:/sys/devices/pci0000:00/0000:00:02.1/usb1/1-7/1-7:1.0/firmware/1-7:1.0/loading CPU 3
Modules linked in: af_packet
cpufreq_conservative cpufreq_ondemand cpufreq_userspace cpufreq_powersave
powernow_k8 freq_table xt_NOTRACK ipt_REJECT xt_state iptable_raw
iptable_filter nf_conntrack_netbios_ns nf_conntrack_ipv4 nf_conntrack
nf_defrag_ipv4 ip_tables ip6_tables uhci_hcd snd_hda_codec_realtek rt73usb
rt2x00usb rt2x00lib snd_hda_intel ohci1394 snd_hda_codec led_class
ieee1394 input_polldev snd_pcm mac80211 snd_timer rtc_cmos snd ppdev
button forcedeth pcspkr firewire_ohci soundcore i2c_nforce2 rtc_core
rtc_lib parport_pc cfg80211 parport sr_mod snd_page_alloc i2c_core cdrom
sg usbhid ohci_hcd ehci_hcd sd_mod usbcore hmac loop ecb arc4 fuse
edd ext3 jbd fan pata_amd sata_nv libata scsi_mod thermal processor
Pid: 2362, comm: phy0 Not tainted 2.6.30-rc5-git5 #1
RIP: 0010:[<ffffffffa0273b0f>][<ffffffffa0273b0f>] minstrel_alloc_sta+0x6f/0xf0 [mac80211]
RSP: 0018:ffff88022ddc7b90 EFLAGS: 00010206
RAX: 000000000000000c RBX: ffff88022c150260 RCX: ffff88022c1500c0
RDX: 0000000000000000 RSI: 0000000000008020 RDI: ffff88022b528740
RBP: ffff88022b5286c0 R08: 0000000000000000 R09: 0000000000000058
R10: 000000000000000c R11: ffff88022ddc7cd0 R12: 0000000000008020
R13: 0000000000000020 R14: 0000000000000020 R15: 0000000000000000
FS: 00007f6fc24ec6f0(0000) GS:ffff88002807f000(0000) knlGS:0000000000000000
CS: 0010 DS: 0018 ES: 0018 CR0: 000000008005003b
CR2: 0000000000000018 CR3: 0000000229dff000 CR4: 00000000000006e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process phy0 (pid: 2362, threadinfo ffff88022ddc6000, task ffff88022c512cd0)
Stack: May 15 19:48:40 liesel kernel:0000000000000000 ffff88022c555000 ffff88022c150260 ffff88022f126880
ffff88022f126600 ffffffffa025c62b ffff88022f126600 ffff88022c150260
0000000000000053 ffff880229e18044 ffff88022f126880 ffffffffa0264670
Call Trace:
[<ffffffffa025c62b>] ? sta_info_alloc+0x8b/0x140 [mac80211]
[<ffffffffa0264670>] ? ieee80211_rx_mgmt_assoc_resp+0xa20/0xb90 [mac80211]
[<ffffffffa026f1d1>] ? __ieee80211_tx+0x61/0xd0 [mac80211]
[<ffffffffa026f34d>] ? ieee80211_tx+0x10d/0x270 [mac80211]
[<ffffffff8055d25a>] ? thread_return+0x3e/0x6a4
[<ffffffffa02651f2>] ? ieee80211_sta_work+0xe2/0xab0 [mac80211]
[<ffffffff80254246>] ? queue_work+0x26/0x60
[<ffffffffa0265110>] ? ieee80211_sta_work+0x0/0xab0 [mac80211]
[<ffffffff80253631>] ? worker_thread+0x141/0x230
[<ffffffff80257c00>] ? autoremove_wake_function+0x0/0x30
[<ffffffff802534f0>] ? worker_thread+0x0/0x230
[<ffffffff802534f0>] ? worker_thread+0x0/0x230
[<ffffffff802577e4>] ? kthread+0x54/0x90
[<ffffffff8020ce2a>] ? child_rip+0xa/0x20
[<ffffffff80257790>] ? kthread+0x0/0x90
[<ffffffff8020ce20>] ? child_rip+0x0/0x20
Code: 89 c5 31 c0 48 85 ed 74 6c 48 8b 4b
28 31 c0 41 b9 58 00 00 00 44 89 e6 48 8b 51 20 44 8b 52 18 45 85 d2 0f 49
42 18 48 8b 51 28 <39> 42 18 89 c3 0f 4d 5a 18 48 63 fb 49 0f af f9 e8 dc
4e 04 e0
RIP [<ffffffffa0273b0f>] minstrel_alloc_sta+0x6f/0xf0 [mac80211]
RSP <ffff88022ddc7b90>
CR2: 0000000000000018
---[ end trace 7489e902c4428832 ]---
ifup-dhcp: .
syslog-ng[3073]: last message repeated 11 times
ifup-dhcp: no IP address yet... backgrounding.
[....]
Regards,
Heinz.
On Fri, May 15, 2009 at 11:49:04AM -0700, Linus Torvalds wrote:
>
>
> On Fri, 15 May 2009, John W. Linville wrote:
> >
> > Unfortunately, I think Dave might have limited availability right now...
>
> Yeah, I think he's on some Alaskan cruise.
>
> Should I just pull directly from you this time?
I think that is the best option.
Thanks!
John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
On Fri, 15 May 2009, Marcel Holtmann wrote:
> Hi Linus,
>
> > > Unfortunately, I think Dave might have limited availability right now...
> >
> > Yeah, I think he's on some Alaskan cruise.
> >
> > Should I just pull directly from you this time?
>
> I would have a similar pull request pending for Bluetooth fixes:
>
> http://marc.info/?l=linux-netdev&m=124209765506433
Ok, I pulled that one too.
Linus
On Mon, May 4, 2009 at 12:04 PM, Jiri Slaby <[email protected]> wrote:
> minstrel doesn't count max rate count in fact, since it doesn't use
> a loop variable `i' and hence allocs space only for bitrates found in
> the first band.
Nice catch, I don't know how many times I read that and didn't see
it. Hopefully that has some bearing on this long standing bug:
http://bugzilla.kernel.org/show_bug.cgi?id=12490
--
Bob Copeland %% http://www.bobcopeland.com
Jiri Slaby wrote:
> minstrel doesn't count max rate count in fact, since it doesn't use
> a loop variable `i' and hence allocs space only for bitrates found in
> the first band.
>
> Fix it by involving the `i' as an index so that it traverses all the
> bands now and finds the real max bitrate count.
>
> Signed-off-by: Jiri Slaby <[email protected]>
> Cc: Felix Fietkau <[email protected]>
Good catch!
Acked-by: Felix Fietkau <[email protected]>
On Fri, May 15, 2009 at 08:21:31PM +0200, Heinz Diehl wrote:
> On 04.05.2009, Jiri Slaby wrote:
>
> > minstrel doesn't count max rate count in fact, since it doesn't use
> > a loop variable `i' and hence allocs space only for bitrates found in
> > the first band.
> [....]
>
> This patchset crashes my WLAN. Reverting it does fix this:
http://www.kernel.org/pub/linux/kernel/people/linville/wireless-2.6/0002-mac80211-avoid-NULL-ptr-deref-when-finding-max_rate.patch
Original pull request here:
http://marc.info/?l=linux-kernel&m=124214056932143&w=2
Unfortunately, I think Dave might have limited availability right now...
John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
On Mon, 2009-05-04 at 18:04 +0200, Jiri Slaby wrote:
> + for (i = 0; i < IEEE80211_NUM_BANDS; i++) {
> + sband = hw->wiphy->bands[i];
> + if (sband->n_bitrates > max_rates)
> + max_rates = sband->n_bitrates;
> + }
> +
> /* We can safely assume that sband won't change unless we get
> * reinitialized. */
> - rinfo = kmalloc(sizeof(*rinfo) * sband->n_bitrates, GFP_ATOMIC);
> + rinfo = kmalloc(sizeof(*rinfo) * max_rates, GFP_ATOMIC);
I suppose you should remove the comment...
johannes
pid doesn't count with some band having more bitrates than the one
associated the first time.
Fix that by counting the maximal available bitrate count and allocate
big enough space.
Secondly, fix touching uninitialized memory which causes panics.
Index sucked from this random memory points to the hell.
The fix is to sort the rates on each band change.
[v2]
- remove comment which is wrong now
Signed-off-by: Jiri Slaby <[email protected]>
---
net/mac80211/rc80211_pid_algo.c | 73 +++++++++++++++++++++------------------
1 files changed, 39 insertions(+), 34 deletions(-)
diff --git a/net/mac80211/rc80211_pid_algo.c b/net/mac80211/rc80211_pid_algo.c
index b16801c..01d59a8 100644
--- a/net/mac80211/rc80211_pid_algo.c
+++ b/net/mac80211/rc80211_pid_algo.c
@@ -317,13 +317,44 @@ rate_control_pid_rate_init(void *priv, struct ieee80211_supported_band *sband,
struct ieee80211_sta *sta, void *priv_sta)
{
struct rc_pid_sta_info *spinfo = priv_sta;
+ struct rc_pid_info *pinfo = priv;
+ struct rc_pid_rateinfo *rinfo = pinfo->rinfo;
struct sta_info *si;
+ int i, j, tmp;
+ bool s;
/* TODO: This routine should consider using RSSI from previous packets
* as we need to have IEEE 802.1X auth succeed immediately after assoc..
* Until that method is implemented, we will use the lowest supported
* rate as a workaround. */
+ /* Sort the rates. This is optimized for the most common case (i.e.
+ * almost-sorted CCK+OFDM rates). Kind of bubble-sort with reversed
+ * mapping too. */
+ for (i = 0; i < sband->n_bitrates; i++) {
+ rinfo[i].index = i;
+ rinfo[i].rev_index = i;
+ if (RC_PID_FAST_START)
+ rinfo[i].diff = 0;
+ else
+ rinfo[i].diff = i * pinfo->norm_offset;
+ }
+ for (i = 1; i < sband->n_bitrates; i++) {
+ s = 0;
+ for (j = 0; j < sband->n_bitrates - i; j++)
+ if (unlikely(sband->bitrates[rinfo[j].index].bitrate >
+ sband->bitrates[rinfo[j + 1].index].bitrate)) {
+ tmp = rinfo[j].index;
+ rinfo[j].index = rinfo[j + 1].index;
+ rinfo[j + 1].index = tmp;
+ rinfo[rinfo[j].index].rev_index = j;
+ rinfo[rinfo[j + 1].index].rev_index = j + 1;
+ s = 1;
+ }
+ if (!s)
+ break;
+ }
+
spinfo->txrate_idx = rate_lowest_index(sband, sta);
/* HACK */
si = container_of(sta, struct sta_info, sta);
@@ -336,21 +367,22 @@ static void *rate_control_pid_alloc(struct ieee80211_hw *hw,
struct rc_pid_info *pinfo;
struct rc_pid_rateinfo *rinfo;
struct ieee80211_supported_band *sband;
- int i, j, tmp;
- bool s;
+ int i, max_rates = 0;
#ifdef CONFIG_MAC80211_DEBUGFS
struct rc_pid_debugfs_entries *de;
#endif
- sband = hw->wiphy->bands[hw->conf.channel->band];
-
pinfo = kmalloc(sizeof(*pinfo), GFP_ATOMIC);
if (!pinfo)
return NULL;
- /* We can safely assume that sband won't change unless we get
- * reinitialized. */
- rinfo = kmalloc(sizeof(*rinfo) * sband->n_bitrates, GFP_ATOMIC);
+ for (i = 0; i < IEEE80211_NUM_BANDS; i++) {
+ sband = hw->wiphy->bands[i];
+ if (sband->n_bitrates > max_rates)
+ max_rates = sband->n_bitrates;
+ }
+
+ rinfo = kmalloc(sizeof(*rinfo) * max_rates, GFP_ATOMIC);
if (!rinfo) {
kfree(pinfo);
return NULL;
@@ -368,33 +400,6 @@ static void *rate_control_pid_alloc(struct ieee80211_hw *hw,
pinfo->rinfo = rinfo;
pinfo->oldrate = 0;
- /* Sort the rates. This is optimized for the most common case (i.e.
- * almost-sorted CCK+OFDM rates). Kind of bubble-sort with reversed
- * mapping too. */
- for (i = 0; i < sband->n_bitrates; i++) {
- rinfo[i].index = i;
- rinfo[i].rev_index = i;
- if (RC_PID_FAST_START)
- rinfo[i].diff = 0;
- else
- rinfo[i].diff = i * pinfo->norm_offset;
- }
- for (i = 1; i < sband->n_bitrates; i++) {
- s = 0;
- for (j = 0; j < sband->n_bitrates - i; j++)
- if (unlikely(sband->bitrates[rinfo[j].index].bitrate >
- sband->bitrates[rinfo[j + 1].index].bitrate)) {
- tmp = rinfo[j].index;
- rinfo[j].index = rinfo[j + 1].index;
- rinfo[j + 1].index = tmp;
- rinfo[rinfo[j].index].rev_index = j;
- rinfo[rinfo[j + 1].index].rev_index = j + 1;
- s = 1;
- }
- if (!s)
- break;
- }
-
#ifdef CONFIG_MAC80211_DEBUGFS
de = &pinfo->dentries;
de->target = debugfs_create_u32("target_pf", S_IRUSR | S_IWUSR,
--
1.6.2.4
On 05/04/2009 08:38 PM, Luis R. Rodriguez wrote:
> Cc: [email protected]
Sure, I plan to send a notification to Greg after a SHA from the linus
tree will be known.
Thanks.