Skip to contentSkip to conversation

softmac: Fix deadlock of wx_set_essid with assoc work

PATCH In review Michael Buesch 1 participant 2 messages Aug 7, 2007 Networking

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 softmac: Fix deadlock of wx_set_essid with assoc work

The essid wireless extension does deadlock against the assoc mutex, as we don't unlock the assoc mutex.

--

This is included in John's wireless-2.6 tree.

This doesn't fix the whole issue completely, as we shouldn't flush the workqueue while we are holding the rtnl lock. But, it turns this bug from a "will 100% always deadlock" into a "it might deadlock" bug.
A followup patch will come (if someone has some time to do so) to fix this crap code completely.

net/ieee80211/softmac/ieee80211softmac_wx.c +4 -1
Index: bu3sch-wireless-dev/net/ieee80211/softmac/ieee80211softmac_wx.c
===================================================================
@@ -98,9 +98,12 @@ ieee80211softmac_wx_set_essid(struct net
 				cancel_delayed_work(&authptr->work);
 			sm->associnfo.bssvalid = 0;
 			sm->associnfo.bssfixed = 0;
-			flush_scheduled_work();
 			sm->associnfo.associating = 0;
 			sm->associnfo.associated = 0;
+			/* Unlock mutex, otherwise we deadlock with the assoc work handler. */
+			mutex_unlock(&sm->associnfo.mutex);
+			flush_scheduled_work();
+			mutex_lock(&sm->associnfo.mutex);
 		}
 	}
 
Signature · 1 line
Greetings Michael.
#1 lore raw
Quoted 26 lines · On Tuesday 07 August 2007 12:02:58 Michael Buesch wrote:

The essid wireless extension does deadlock against the assoc mutex, as we don't unlock the assoc mutex.

--

This is included in John's wireless-2.6 tree.

This doesn't fix the whole issue completely, as we shouldn't flush the workqueue while we are holding the rtnl lock. But, it turns this bug from a "will 100% always deadlock" into a "it might deadlock" bug.
A followup patch will come (if someone has some time to do so) to fix this crap code completely.

Index: bu3sch-wireless-dev/net/ieee80211/softmac/ieee80211softmac_wx.c
===================================================================

net/ieee80211/softmac/ieee80211softmac_wx.c +0 -0
@@ -98,9 +98,12 @@ ieee80211softmac_wx_set_essid(struct net
 				cancel_delayed_work(&authptr->work);
 			sm->associnfo.bssvalid = 0;
 			sm->associnfo.bssfixed = 0;

net/ieee80211/softmac/ieee80211softmac_wx.c ieee80211softmac_wx_set_essid(struct net · line 109

Quoted 10 lines
net/ieee80211/softmac/ieee80211softmac_wx.c +4 -1
-			flush_scheduled_work();
 			sm->associnfo.associating = 0;
 			sm->associnfo.associated = 0;
+			/* Unlock mutex, otherwise we deadlock with the assoc work handler. */
+			mutex_unlock(&sm->associnfo.mutex);
+			flush_scheduled_work();
+			mutex_lock(&sm->associnfo.mutex);
 		}
 	}
 

Whoops, no. I attached the wrong patch. Sorry!

Signature · 1 line
Greetings Michael.