Skip to contentSkip to conversation

b43: Don't abuse wl->current_dev in the led work

PATCH In review Michael Buesch 2 participants 4 messages Sep 14, 2009 – Sep 15, 2009 Networking drivers (wireless)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 b43: Don't abuse wl->current_dev in the led work

Don't abuse wl->current_dev in the LED work for checking whether we're going down. Add an explicit variable.
This fixes a crash on rmmod dereferencing the wl->current_dev NULL pointer in various other places of the driver.

---

Note to self: Don't try to safe a byte of memory :)

drivers/net/wireless/b43/leds.c +3 -4
Index: wireless-testing/drivers/net/wireless/b43/leds.c
===================================================================
@@ -112,10 +112,7 @@ static void b43_led_brightness_set(struc
 	struct b43_led *led = container_of(led_dev, struct b43_led, led_dev);
 	struct b43_wl *wl = led->wl;
 
-	/* The check for current_dev is only needed while unregistering,
-	 * so it is sequencial and does not race. But we must not dereference
-	 * current_dev here. */
-	if (likely(wl->current_dev)) {
+	if (likely(!wl->leds.stop)) {
 		atomic_set(&led->state, brightness);
 		ieee80211_queue_work(wl->hw, &wl->leds.work);
 	}
@@ -314,6 +311,8 @@ void b43_leds_init(struct b43_wldev *dev
 			break;
 		}
 	}
+
+	dev->wl->leds.stop = 0;
 }
 
 void b43_leds_exit(struct b43_wldev *dev)
drivers/net/wireless/b43/leds.h +1 -0
Index: wireless-testing/drivers/net/wireless/b43/leds.h
===================================================================
@@ -35,6 +35,7 @@ struct b43_leds {
 	struct b43_led led_radio;
 	struct b43_led led_assoc;
 
+	bool stop;
 	struct work_struct work;
 };
 
drivers/net/wireless/b43/main.c +1 -1
Index: wireless-testing/drivers/net/wireless/b43/main.c
===================================================================
@@ -4987,7 +4987,7 @@ static void b43_remove(struct ssb_device
 		 * might have modified it. Restoring is important, so the networking
 		 * stack can properly free resources. */
 		wl->hw->queues = wl->mac80211_initially_registered_queues;
-		wl->current_dev = NULL;
+		wl->leds.stop = 1;
 		cancel_work_sync(&wl->leds.work);
 		ieee80211_unregister_hw(wl->hw);
 	}
Signature · 1 line
Greetings, Michael.
#1 lore raw
Quoted 6 lines · Michael Buesch

Don't abuse wl->current_dev in the LED work for checking whether we're going down. Add an explicit variable.
This fixes a crash on rmmod dereferencing the wl->current_dev NULL pointer in various other places of the driver.

This patch does not apply. What other patches should I have beyond the current state of wireless-testing?

Larry

#2 lore raw
Quoted 10 lines · On Tuesday 15 September 2009 00:58:18 Larry Finger wrote:
Quoted 6 lines · Michael Buesch

Don't abuse wl->current_dev in the LED work for checking whether we're going down. Add an explicit variable.
This fixes a crash on rmmod dereferencing the wl->current_dev NULL pointer in various other places of the driver.

This patch does not apply. What other patches should I have beyond the current state of wireless-testing?

All patches that are currently queued up.

Signature · 1 line
Greetings, Michael.
#3 lore raw
Quoted 7 lines · Michael Buesch
Quoted 6 lines · On Tuesday 15 September 2009 00:58:18 Larry Finger wrote:
Quoted 5 lines · Michael Buesch

Don't abuse wl->current_dev in the LED work for checking whether we're going down. Add an explicit variable.
This fixes a crash on rmmod dereferencing the wl->current_dev NULL pointer in various other places of the driver.

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

This patch does not apply. What other patches should I have beyond the current state of wireless-testing?

All patches that are currently queued up.

I found the ones I needed in my mailboxes.

The patch cured my problem. Thanks and ack.

Larry