LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jarek Poplawski <jarkao2@gmail.com>
To: David Miller <davem@davemloft.net>
Cc: Folkert van Heusden <folkert@vanheusden.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Subject: [PATCH] Re: [2.6.26] OOPS in __linkwatch_run_queue (unable to handle kernel NULL pointer dereference at 00000235)
Date: Mon, 17 Nov 2008 08:40:58 +0000	[thread overview]
Message-ID: <20081117084058.GA6345@ff.dom.local> (raw)
In-Reply-To: <20081030020910.f97ee3fc.akpm@linux-foundation.org>

On 30-10-2008 10:09, Andrew Morton wrote:
> (cc netdev)
> 
> On Mon, 27 Oct 2008 16:00:02 +0100 Folkert van Heusden <folkert@vanheusden.com> wrote:
> 
>> Hi,                                                                                                                                                                                                
>>                                                                                                                                                                                                    
>> While running my http://vanheusden.com/pyk/ script (which randomly
>> inserts and removes modules) I triggered the folllowing oops in a 2.6.26
>> kernel on a pre-ht pentium 4 (hp pavillion laptop type zv5231ea):
>>
>> [ 1037.480097] BUG: unable to handle kernel NULL pointer dereference at 00000235
>> [ 1037.480188] IP: [<c0261078>] __linkwatch_run_queue+0x6d/0x15a
...

------------------->

net: link_watch: Don't add a linkwatch event before register_netdev()

b44 and some other network drivers run netif_carrier_off() before
register_netdev(). Then, if register fails, free_netdev() destruction
is done while dev is still referenced and held on the lweventlist.

Of course, it would be nice if all drivers could use some common order
of calling things like register_netdev() vs. netif_carrier_off(), but
since there is a lot of this I guess there is probably some reason,
so this patch doesn't change the order but assumes that such an early
netif_carrier_off() is only to set the __LINK_STATE_NOCARRIER flag,
and some netif_carrier_on()/_off() will still follow.

Reported-by: Folkert van Heusden <folkert@vanheusden.com>
Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
---

diff --git a/net/core/link_watch.c b/net/core/link_watch.c
index bf8f7af..393c2ba 100644
--- a/net/core/link_watch.c
+++ b/net/core/link_watch.c
@@ -216,8 +216,13 @@ void linkwatch_fire_event(struct net_device *dev)
 	bool urgent = linkwatch_urgent_event(dev);
 
 	if (!test_and_set_bit(__LINK_STATE_LINKWATCH_PENDING, &dev->state)) {
-		dev_hold(dev);
+		/* don't add an event before register_netdev(); it can fail */
+		if (!test_bit(__LINK_STATE_PRESENT, &dev->state)) {
+			WARN_ON(1);
+			return;
+		}
 
+		dev_hold(dev);
 		linkwatch_add_event(dev);
 	} else if (!urgent)
 		return;
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 80c8f3d..8f99c06 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -286,7 +286,8 @@ EXPORT_SYMBOL(netif_carrier_on);
 void netif_carrier_off(struct net_device *dev)
 {
 	if (!test_and_set_bit(__LINK_STATE_NOCARRIER, &dev->state))
-		linkwatch_fire_event(dev);
+		if (test_bit(__LINK_STATE_PRESENT, &dev->state))
+			linkwatch_fire_event(dev);
 }
 EXPORT_SYMBOL(netif_carrier_off);
 

  reply	other threads:[~2008-11-17  8:41 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-27 15:00 Folkert van Heusden
2008-10-30  9:09 ` Andrew Morton
2008-11-17  8:40   ` Jarek Poplawski [this message]
2008-11-17  8:50     ` [PATCH] " David Miller
2008-11-17  9:11       ` Jarek Poplawski
2008-11-17  9:15         ` David Miller
2008-11-19 23:35           ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20081117084058.GA6345@ff.dom.local \
    --to=jarkao2@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=folkert@vanheusden.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --subject='[PATCH] Re: [2.6.26] OOPS in __linkwatch_run_queue (unable to handle kernel NULL pointer dereference at 00000235)' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).