LKML Archive on
help / color / mirror / Atom feed
From: David Miller <>
Subject: Re: [PATCH] Re: [2.6.26] OOPS in __linkwatch_run_queue (unable to handle kernel NULL pointer dereference at 00000235)
Date: Mon, 17 Nov 2008 00:50:25 -0800 (PST)	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <20081117084058.GA6345@ff.dom.local>

From: Jarek Poplawski <>
Date: Mon, 17 Nov 2008 08:40:58 +0000

> 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 <>
> Signed-off-by: Jarek Poplawski <>

Ugh, drivers should not be fiddling with stuff like this before the
device object is even registered.

I can just imagine all sorts of other operations drivers might find
it "convenient" to do before the sucker is even registered, causing
similar if not even worse problems.

It's pretty simple, make netif_carrier_off() be the first thing
->open() does and make it the last thing ->stop() and ->suspend()
do.  That's how to fix this bug.

I'm going to fix this as follows.  B44 already abided by 2/3 of
this by handling the ->stop() and ->suspend() cases correctly already.

b44: Do not call netif_carrier_off() before device is even registered.

This is illegal.  Instead simply do this first thing in ->open().

Signed-off-by: David S. Miller <>

diff --git a/drivers/net/b44.c b/drivers/net/b44.c
index c3bda5c..2e353b8 100644
--- a/drivers/net/b44.c
+++ b/drivers/net/b44.c
@@ -1426,6 +1426,8 @@ static int b44_open(struct net_device *dev)
 	struct b44 *bp = netdev_priv(dev);
 	int err;
+	netif_carrier_off(dev);
 	err = b44_alloc_consistent(bp, GFP_KERNEL);
 	if (err)
 		goto out;
@@ -2165,8 +2167,6 @@ static int __devinit b44_init_one(struct ssb_device *sdev,
 	dev->irq = sdev->irq;
 	SET_ETHTOOL_OPS(dev, &b44_ethtool_ops);
-	netif_carrier_off(dev);
 	err = ssb_bus_powerup(sdev->bus, 0);
 	if (err) {

  reply	other threads:[~2008-11-17  8:50 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   ` [PATCH] " Jarek Poplawski
2008-11-17  8:50     ` David Miller [this message]
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:

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

  git send-email \ \ \ \ \ \ \ \
    --subject='Re: [PATCH] Re: [2.6.26] OOPS in __linkwatch_run_queue (unable to handle kernel NULL pointer dereference at 00000235)' \

* 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).