[PATCH 2.6.19-git19] BUG due to bad argument to ieee80211softmac_assoc_work

[PATCH 2.6.19-git19] BUG due to bad argument to ieee80211softmac_assoc_work

[Michael Bommarito]

This didn't get much attention on bugzilla and I figured it was
important enough to forward along to the whole list since it's been
lingering around in ieee80211-softmac since 19-git5 at least.
http://bugzilla.kernel.org/show_bug.cgi?id=7657

Somebody was passing the whole mac device structure to
ieee80211softmac_assoc_work instead of just the assocation work, which
lead to much death and locking.

Attached is a patch that fixes this (the actual change is two lines
but context provided in patch for review).  The dmesg containing call
trace is attached to the bugzilla entry above.

-Mike

Re: [PATCH 2.6.19-git19] BUG due to bad argument to ieee80211softmac_assoc_work

[Michael Bommarito]

[-- Attachment #1: Type: text/plain, Size: 1191 bytes --]

Sorry, realized I might not have been clear as to what I meant!  The
patch was attached to the bugzilla entry, but I'll attach it here as
well.  My description of the patch itself was really as complicated as
it gets too (just two lines, switch (void*)mac to
&mac->assoc.work.work in
net/ieee80211/softmac/ieee80211softmac_assoc.c), just a small bug
while somebody was rushing through the work/delayed_work changes.

-Mike

On 12/13/06, Mike Galbraith <efault@gmx.de> wrote:
> On Wed, 2006-12-13 at 13:17 -0500, Michael Bommarito wrote:
> > This didn't get much attention on bugzilla and I figured it was
> > important enough to forward along to the whole list since it's been
> > lingering around in ieee80211-softmac since 19-git5 at least.
> > http://bugzilla.kernel.org/show_bug.cgi?id=7657
> >
> > Somebody was passing the whole mac device structure to
> > ieee80211softmac_assoc_work instead of just the assocation work, which
> > lead to much death and locking.
> >
> > Attached is a patch that fixes this (the actual change is two lines
> > but context provided in patch for review).  The dmesg containing call
> > trace is attached to the bugzilla entry above.
>
> -ENOPATCH :)
>
>

[-- Attachment #2: patch-ieee80211softmac_assoc_work --]
[-- Type: application/octet-stream, Size: 766 bytes --]

--- net/ieee80211/softmac/ieee80211softmac_assoc.c	2006-12-13 11:23:03.000000000 -0500
+++ net/ieee80211/softmac/ieee80211softmac_assoc.c	2006-12-13 11:24:26.000000000 -0500
@@ -167,7 +167,7 @@
 ieee80211softmac_assoc_notify_scan(struct net_device *dev, int event_type, void *context)
 {
 	struct ieee80211softmac_device *mac = ieee80211_priv(dev);
-	ieee80211softmac_assoc_work((void*)mac);
+	ieee80211softmac_assoc_work(&mac->associnfo.work.work);
 }
 
 static void
@@ -177,7 +177,7 @@
 
 	switch (event_type) {
 	case IEEE80211SOFTMAC_EVENT_AUTHENTICATED:
-		ieee80211softmac_assoc_work((void*)mac);
+	        ieee80211softmac_assoc_work(&mac->associnfo.work.work);
 		break;
 	case IEEE80211SOFTMAC_EVENT_AUTH_FAILED:
 	case IEEE80211SOFTMAC_EVENT_AUTH_TIMEOUT:

Re: [PATCH 2.6.19-git19] BUG due to bad argument to ieee80211softmac_assoc_work

[Ray Lee]

On 12/13/06, Michael Bommarito <michael.bommarito@gmail.com> wrote:
> Sorry, realized I might not have been clear as to what I meant!  The
> patch was attached to the bugzilla entry, but I'll attach it here as
> well.  My description of the patch itself was really as complicated as
> it gets too (just two lines, switch (void*)mac to
> &mac->assoc.work.work in
> net/ieee80211/softmac/ieee80211softmac_assoc.c), just a small bug
> while somebody was rushing through the work/delayed_work changes.

--- net/ieee80211/softmac/ieee80211softmac_assoc.c	2006-12-13
11:23:03.000000000 -0500
+++ net/ieee80211/softmac/ieee80211softmac_assoc.c	2006-12-13
11:24:26.000000000 -0500
@@ -167,7 +167,7 @@
 ieee80211softmac_assoc_notify_scan(struct net_device *dev, int
event_type, void *context)
 {
 	struct ieee80211softmac_device *mac = ieee80211_priv(dev);
-	ieee80211softmac_assoc_work((void*)mac);
+	ieee80211softmac_assoc_work(&mac->associnfo.work.work);
 }

 static void
@@ -177,7 +177,7 @@

 	switch (event_type) {
 	case IEEE80211SOFTMAC_EVENT_AUTHENTICATED:
-		ieee80211softmac_assoc_work((void*)mac);
+	        ieee80211softmac_assoc_work(&mac->associnfo.work.work);
 		break;
 	case IEEE80211SOFTMAC_EVENT_AUTH_FAILED:
 	case IEEE80211SOFTMAC_EVENT_AUTH_TIMEOUT:

Good catch, though it was already caught. See:

   http://lkml.org/lkml/2006/12/12/46

...for (basically) the same patch.

But again, good catch :-).

Re: [PATCH 2.6.19-git19] BUG due to bad argument to ieee80211softmac_assoc_work

[Michael Bommarito]

Ah, apologies, it's exam time and I probably didn't look hard enough
on the mailing list before posting.  For the record though, I'd posted
the bug (no patch) to bugzilla on the 9th (although it looks as if the
email address it was assigned to is actually defunct - anyone know why
bugzilla is still using networking_wireless@kernel-bugs.osdl.org ?)
Anyway, again, sorry for the duplicate!

-Mike

On 12/14/06, Ray Lee <madrabbit@gmail.com> wrote:
> On 12/13/06, Michael Bommarito <michael.bommarito@gmail.com> wrote:
> > Sorry, realized I might not have been clear as to what I meant!  The
> > patch was attached to the bugzilla entry, but I'll attach it here as
> > well.  My description of the patch itself was really as complicated as
> > it gets too (just two lines, switch (void*)mac to
> > &mac->assoc.work.work in
> > net/ieee80211/softmac/ieee80211softmac_assoc.c), just a small bug
> > while somebody was rushing through the work/delayed_work changes.
>
> --- net/ieee80211/softmac/ieee80211softmac_assoc.c      2006-12-13
> 11:23:03.000000000 -0500
> +++ net/ieee80211/softmac/ieee80211softmac_assoc.c      2006-12-13
> 11:24:26.000000000 -0500
> @@ -167,7 +167,7 @@
>  ieee80211softmac_assoc_notify_scan(struct net_device *dev, int
> event_type, void *context)
>  {
>         struct ieee80211softmac_device *mac = ieee80211_priv(dev);
> -       ieee80211softmac_assoc_work((void*)mac);
> +       ieee80211softmac_assoc_work(&mac->associnfo.work.work);
>  }
>
>  static void
> @@ -177,7 +177,7 @@
>
>         switch (event_type) {
>         case IEEE80211SOFTMAC_EVENT_AUTHENTICATED:
> -               ieee80211softmac_assoc_work((void*)mac);
> +               ieee80211softmac_assoc_work(&mac->associnfo.work.work);
>                 break;
>         case IEEE80211SOFTMAC_EVENT_AUTH_FAILED:
>         case IEEE80211SOFTMAC_EVENT_AUTH_TIMEOUT:
>
> Good catch, though it was already caught. See:
>
>    http://lkml.org/lkml/2006/12/12/46
>
> ...for (basically) the same patch.
>
> But again, good catch :-).
>

Re: [PATCH 2.6.19-git19] BUG due to bad argument to ieee80211softmac_assoc_work

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 348 bytes --]

On Wed, 2006-12-13 at 13:17 -0500, Michael Bommarito wrote:

> Attached is a patch that fixes this (the actual change is two lines
> but context provided in patch for review).  The dmesg containing call
> trace is attached to the bugzilla entry above.

You forgot to attach the patch but IIRC it's been found and fixed
already.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: [PATCH 2.6.19-git19] BUG due to bad argument to ieee80211softmac_assoc_work

[Uli Kunitz]

Michael,

I sent a patch to this list on Sunday, that patched the problem. It  
seems to be migrated into the wireless-2.6 git tree.

Regards,

Uli
Am 13.12.2006 um 19:17 schrieb Michael Bommarito:

> This didn't get much attention on bugzilla and I figured it was
> important enough to forward along to the whole list since it's been
> lingering around in ieee80211-softmac since 19-git5 at least.
> http://bugzilla.kernel.org/show_bug.cgi?id=7657
>
> Somebody was passing the whole mac device structure to
> ieee80211softmac_assoc_work instead of just the assocation work, which
> lead to much death and locking.
>
> Attached is a patch that fixes this (the actual change is two lines
> but context provided in patch for review).  The dmesg containing call
> trace is attached to the bugzilla entry above.
>
> -Mike
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Uli Kunitz

Re: [PATCH 2.6.19-git19] BUG due to bad argument to ieee80211softmac_assoc_work

[Michael Bommarito]

Hello Uli,
  Yes, apologies, I had been waiting for an abandoned bugzilla entry
to get attention, and when I realized it was assigned to a dead-end, I
had simply posted the patch without checking for prior messages.
  I was further confused by the fact that it hadn't made its way into
any of the 19-gitX sets (and for that matter, the window for
2.6.20-rc1 has come and gone and this still remains unfixed), despite
how clear the error was and how trivial the fix seems.

-Mike

On 12/14/06, Uli Kunitz <kune@deine-taler.de> wrote:
> Michael,
>
> I sent a patch to this list on Sunday, that patched the problem. It
> seems to be migrated into the wireless-2.6 git tree.
>
> Regards,
>
> Uli
> Am 13.12.2006 um 19:17 schrieb Michael Bommarito:
>
> > This didn't get much attention on bugzilla and I figured it was
> > important enough to forward along to the whole list since it's been
> > lingering around in ieee80211-softmac since 19-git5 at least.
> > http://bugzilla.kernel.org/show_bug.cgi?id=7657
> >
> > Somebody was passing the whole mac device structure to
> > ieee80211softmac_assoc_work instead of just the assocation work, which
> > lead to much death and locking.
> >
> > Attached is a patch that fixes this (the actual change is two lines
> > but context provided in patch for review).  The dmesg containing call
> > trace is attached to the bugzilla entry above.
> >
> > -Mike
> > -
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> Uli Kunitz
>
>
>
>

Re: [PATCH 2.6.19-git19] BUG due to bad argument to ieee80211softmac_assoc_work

[Larry Finger]

Michael Bommarito wrote:
> Hello Uli,
>  Yes, apologies, I had been waiting for an abandoned bugzilla entry
> to get attention, and when I realized it was assigned to a dead-end, I
> had simply posted the patch without checking for prior messages.
>  I was further confused by the fact that it hadn't made its way into
> any of the 19-gitX sets (and for that matter, the window for
> 2.6.20-rc1 has come and gone and this still remains unfixed), despite
> how clear the error was and how trivial the fix seems.

I was not aware that a bugzilla entry existed for this problem. I learned about it when my system 
would hang on bootup if the bcm43xx card was installed. By bisection, I learned which commit was 
causing the problem. About that time, the complete fix was discussed on the netdev and bcm43xx 
mailing lists. I was a little perturbed that only part of the fix was accepted into 2.6.19-gitX.

The full fix was pushed to John Linville on Dec. 10, who pushed it on to Jeff Garzik on Dec. 11. I 
have not yet seen any message sending it on to Andrew Morton or Linus.

A bug fix will always be accepted, particularly one that only changes 2 lines - it is only a new 
feature that will no longer be accepted once the -rc1 stage is reached. If this message doesn't do 
the trick and it isn't included by -rc2, I'll ping Jeff to see what happened. Changes always take 
longer than one likes, but one needs to be careful.

Larry