Patchwork Fix caif BUG() with network namespaces

login
register
mail settings
Submitter David Woodhouse
Date Oct. 25, 2011, 7:25 a.m.
Message ID <1319527522.24797.10.camel@shinybook.infradead.org>
Download mbox | patch
Permalink /patch/121504/
State Accepted
Delegated to: David Miller
Headers show

Comments

David Woodhouse - Oct. 25, 2011, 7:25 a.m.
The caif code will register its own pernet_operations, and then register
a netdevice_notifier. Each time the netdevice_notifier is triggered,
it'll do some stuff... including a lookup of its own pernet stuff with
net_generic().

If the net_generic() call ever returns NULL, the caif code will BUG().
That doesn't seem *so* unreasonable, I suppose — it does seem like it
should never happen.

However, it *does* happen. When we clone a network namespace,
setup_net() runs through all the pernet_operations one at a time. It
gets to loopback before it gets to caif. And loopback_net_init()
registers a netdevice... while caif hasn't been initialised. So the caif
netdevice notifier triggers, and immediately goes BUG().

We could imagine a complex and overengineered solution to this generic
class of problems, but this patch takes the simple approach. It just
makes caif_device_notify() *not* go looking for its pernet data
structures if the device it's being notified about isn't a caif device
in the first place.

Cc: stable@kernel.org
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Acked-by: Sjur Brændeland <sjur.brandeland@stericsson.com>

---
On Mon, 2011-10-24 at 18:28 -0400, David Miller wrote:
> David W., the copy that ended up in patchwork was using DOS newlines:
> http://patchwork.ozlabs.org/patch/121250/
> Click on Download "mbox" to see what I mean.
> Can you get your email configuration straightened out before
> resubmitting this patch a third time?

Sorry about that. It's not a configuration thing; just me being stupid
and using the wrong account to send. I *know* Exchange is not an email
server, and that signing emails causes QP encoding. This one should be
fine.

Having said that... in fact, email is *supposed* to have DOS newlines.
The SMTP protocol requires it. When receiving on *nix boxes, we tend to
convert CRLF to LF.

I played a little with applying the emails I sent with 'git am', and it
actually copes fine with the original Quoted-Printable email, as it
should. It *almost* copes with the base64 email, but it forgets to do
the CRLF to LF conversion. I'll look at fixing it (in the spirit of 'be
liberal in what you accept)... but of course we should also be
conservative in what we *send*, so here is (hopefully) an unencoded
version. Apologies again for the inconvenience.

/me double-checks that the signature is turned off, and the sending
account is not the Exchange one... <send>



 net/caif/caif_dev.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
David Miller - Oct. 25, 2011, 11:22 p.m.
From: David Woodhouse <dwmw2@infradead.org>

Date: Tue, 25 Oct 2011 09:25:21 +0200

> The caif code will register its own pernet_operations, and then register

> a netdevice_notifier. Each time the netdevice_notifier is triggered,

> it'll do some stuff... including a lookup of its own pernet stuff with

> net_generic().

> 

> If the net_generic() call ever returns NULL, the caif code will BUG().

> That doesn't seem *so* unreasonable, I suppose — it does seem like it

> should never happen.

> 

> However, it *does* happen. When we clone a network namespace,

> setup_net() runs through all the pernet_operations one at a time. It

> gets to loopback before it gets to caif. And loopback_net_init()

> registers a netdevice... while caif hasn't been initialised. So the caif

> netdevice notifier triggers, and immediately goes BUG().

> 

> We could imagine a complex and overengineered solution to this generic

> class of problems, but this patch takes the simple approach. It just

> makes caif_device_notify() *not* go looking for its pernet data

> structures if the device it's being notified about isn't a caif device

> in the first place.

> 

> Cc: stable@kernel.org

> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>

> Acked-by: Sjur Brændeland <sjur.brandeland@stericsson.com>


Applied, thanks.

Patch

diff --git a/net/caif/caif_dev.c b/net/caif/caif_dev.c
index 7f9ac07..47fc8f3 100644
--- a/net/caif/caif_dev.c
+++ b/net/caif/caif_dev.c
@@ -212,8 +212,7 @@  static int caif_device_notify(struct notifier_block *me, unsigned long what,
 	enum cfcnfg_phy_preference pref;
 	enum cfcnfg_phy_type phy_type;
 	struct cfcnfg *cfg;
-	struct caif_device_entry_list *caifdevs =
-	    caif_device_list(dev_net(dev));
+	struct caif_device_entry_list *caifdevs;
 
 	if (dev->type != ARPHRD_CAIF)
 		return 0;
@@ -222,6 +221,8 @@  static int caif_device_notify(struct notifier_block *me, unsigned long what,
 	if (cfg == NULL)
 		return 0;
 
+	caifdevs = caif_device_list(dev_net(dev));
+
 	switch (what) {
 	case NETDEV_REGISTER:
 		caifd = caif_device_alloc(dev);