Please note: This project is no longer active. The website is kept online for historic purposes only.
If you´re looking for a Linux driver for your Atheros WLAN device, you should continue here .

Ticket #1477 (closed defect: fixed)

Opened 12 years ago

Last modified 11 years ago

[patch] A skb_over_panic fix

Reported by: Andrew Lunn <andrew@lunn.ch> Assigned to:
Priority: major Milestone: version 0.9.5
Component: madwifi: 802.11 stack Version: trunk
Keywords: Cc:
Patch is attached: 0 Pending:

Description

Im not sure my fix is correct, since im not that familar with the driver. So i will describe the bug verbosely and then let the experts decide if my fix is the right one.

Im getting a skb_over_panic, called from skb_put() from ieee80211_beacon_update().

Looking at the current SVN version, ieee80211_beacon_update() starts at line 280 of ieee80211_beacon.c. What is happening is that a change channel has been requested. This causes the flag IEEE80211_F_CHANSWITCH to be set. The code at line 478 is responsible for adding a channel change notification to the beacon. The first time the beacon is updated, a 5 byte information element is added to the beacon. To do this skb_put is called at line 504 to make the data in the skb 5 bytes bigger. The counter/flag iv_chanchange_count is set so that the next time few times around the IE is left alone in the beacon.

On the third call to ieee80211_beacon_update() the code at line 291 gets involved. The channel change has been sent enought times and now is the time to actually change channel. The counter iv_chanchange_count is set back to zero. The code then finds the new channel it is supposed to change to. This is where the problem starts. In my case, the new channel is not a valid channel. ieee80211_doth_findchan() returns NULL. This is checked for and the function just returns. It leaves the flag IEEE80211_F_CHANSWITCH set, but it has reset iv_chanchange_count. So the next time ieee80211_beacon_update() is called, it adds another channel change notification, ie adds another 5 bytes. And around it goes again, fails to find the channel, adds another 5 bytes, .., .., until skb_put detects that we are trying to put more into the buffer than we allocated space for, it calls skb_over_panic() which does BUG() and we die a horrible death in the interrupt handler......

The patch soon to be attached fixes this. If the findchan() returns a null, we continue to initialize the beacon, which removes the channel change indication. We reset the flags. We just don't perform a channel change.

Andrew

Attachments

ieee80211_beacon.c.diff (1.2 kB) - added by Andrew Lunn <andrew@lunn.ch> on 08/02/07 20:11:24.
madwifi-doth_chanswitch.diff (3.4 kB) - added by mentor on 08/03/07 03:59:13.
Modified patch that includes an initial check

Change History

08/02/07 20:11:24 changed by Andrew Lunn <andrew@lunn.ch>

  • attachment ieee80211_beacon.c.diff added.

08/02/07 20:14:49 changed by Andrew Lunn <andrew@lunn.ch>

Arg!

Did the wiki engine eat my signed-off-by: at the head of the patch? I don't see it when i click on the patch. Anyway, here it is again:

Signed-off-by: Andrew Lunn <andrew@lunn.ch>

08/03/07 01:59:31 changed by mentor

  • priority changed from critical to major.

08/03/07 03:59:13 changed by mentor

  • attachment madwifi-doth_chanswitch.diff added.

Modified patch that includes an initial check

08/03/07 04:00:55 changed by mentor

I've fiddled with the patch to add a check the first time we encounter the channel switch, so we avoid sending announcements if the channel isn't there in the first place.

Testing appreciated.

08/03/07 19:51:25 changed by Andrew Lunn <andrew@lunn.ch>

Works for me.

I did think about adding a check into the change channel IOCTL call. However i don't know if the driver knows all the legal channels early on. It could be the ioctl is called before the hardware has been queried to find out the regularatery domain and what channels are valid. The current behaviour would be to store the channel away until needed. If you validate it and return -EINVAL because the interface is not up/ready yet, it might break something..... However an EINVAL for an invalid channel would give bozoes like me who try to use invalid channels a heads up.

08/04/07 03:55:54 changed by mentor

  • status changed from new to closed.
  • resolution set to fixed.

08/04/07 03:56:05 changed by mentor

  • milestone set to version 0.9.4.

(follow-up: ↓ 8 ) 02/11/08 06:21:51 changed by mrenzmann

  • milestone changed from version 0.9.4 to version 0.9.5.

(in reply to: ↑ 7 ; follow-up: ↓ 9 ) 02/19/08 15:02:34 changed by Jochen

Replying to mrenzmann: Does this mean it's not fixed in 0.9.4? I'm having difficulties with 0.9.4 and I suspect that this bug is what it causes. Looking at the source of 0.9.4, the patch "madwifi-doth_chanswitch.diff" seems to be not applied. Trying to apply it fails for the first hunk @ 289. Applying it manually results in a strange lockup where the cursor is still blinking but the computer not responding. What I do is wlanconfig ath0 create wlandev wifi0 wlanmode ap sudo iwconfig ath0 essid asdf channel 60

(in reply to: ↑ 8 ; follow-up: ↓ 10 ) 02/20/08 06:06:02 changed by mrenzmann

Replying to Jochen:

Does this mean it's not fixed in 0.9.4?

Correct.

(in reply to: ↑ 9 ; follow-up: ↓ 11 ) 02/21/08 14:38:22 changed by Jochen

Thanx. I'd like to point out that applying the patch provided here also prevents madwifi cards (and possibly other) from associating to the created vap. An ipw2200 card however could associate.

(in reply to: ↑ 10 ) 02/26/08 17:20:38 changed by anonymous

Replying to Jochen: Scrap that, this was caused by a different, self-made problem.