[PATCH] b43legacy: Fix a sleep-in-atomic bug in b43legacy_op_bss_info_changed
Kalle Valo <[email protected]>
| Newsgroups | org.infradead.lists.b43-dev,org.kernel.vger.linux-kernel,org.kernel.vger.linux-wireless,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
Michael B?sch <[email protected]> writes: >> > --- a/drivers/net/wireless/broadcom/b43legacy/main.c >> > +++ b/drivers/net/wireless/broadcom/b43legacy/main.c >> > @@ -2859,7 +2859,9 @@ static void b43legacy_op_bss_info_changed(struct ieee80211_hw *hw, >> > b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, 0); >> > >> > if (changed & BSS_CHANGED_BSSID) { >> > + spin_unlock_irqrestore(&wl->irq_lock, flags); >> > b43legacy_synchronize_irq(dev); >> > + spin_lock_irqsave(&wl->irq_lock, flags); >> >> To me this looks like a fragile workaround and not a real fix. You can >> easily add new race conditions with releasing the lock like this. >> > > > I think releasing the lock possibly is fine. It certainly is better than > sleeping with a lock held. Sure, but IMHO in general I think the practise of releasing the lock like this in a middle of function is dangerous as one can easily miss that upper and lower halves of the function are not actually atomic anymore. And in this case that it's under a conditional makes it even worse. -- Kalle Valo