| Message ID | CAJK669ZWJsxnSiyKWFFFUWOA3TaGMY-AmgWuVsYfR=tKQaODgg@mail.gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Jan 24, 2012 at 10:06 AM, Sjur Brændeland <sjurbren@gmail.com> wrote: > Hi Sasha, > >>> Nack, we have to handle other device types than just ARPHDR_CAIF after >>> introducing CAIF over USB/NCM. >> What my patch did was simply move the type check to above the net_generic call, >> it didn't add any new checks - which according to what you said, you'll need to do anyway. > > As I said I, don't think your patch would work. Try to see what happens if > dev->type != ARPHDR_CAIF and caifd != NULL. Then the statement: > > if (caifd == NULL && dev->type != ARPHRD_CAIF) > return 0; > is very different from: > > if (dev->type != ARPHRD_CAIF) > return 0; > ... > if (caifd == NULL) > return 0; Right. > Anyway, another option could be to explicitly check if name space is > initialized, > similar to what net_generic() does,e.g. something like: > > diff --git a/net/caif/caif_dev.c b/net/caif/caif_dev.c > index 673728a..3197bc2 100644 > --- a/net/caif/caif_dev.c > +++ b/net/caif/caif_dev.c > @@ -371,6 +371,13 @@ static int caif_device_notify(struct notifier_block *me, un > struct cflayer *layer, *link_support; > int head_room = 0; > struct caif_device_entry_list *caifdevs; > + int len; > + > + rcu_read_lock(); > + len = rcu_dereference(dev_net(dev)->gen)->len; > + rcu_read_unlock(); > + if (caif_net_id > len) > + return 0; > > cfg = get_cfcnfg(dev_net(dev)); > caifdevs = caif_device_list(dev_net(dev)); We could, in that case we'd just need to handle the case where it was initialized by a device with higher id than CAIF (which we already do I think), and do it without touching net_generic structure directly. btw, Why do we store the devices per-namespace instead of globally? Is it such a big benefit in performance? -- 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
On Tue, Jan 24, 2012 at 5:27 PM, Sjur Brændeland <sjur.brandeland@stericsson.com> wrote: > net_generic() calls BUG_ON() if called with uninitialized > network name-space. Add check if net is initialized before > calling net_generic(). This fixes the following oops: > > [ 200.752016] kernel BUG at include/net/netns/generic.h:40! > ... > [ 200.752016] [<ffffffff825c3cea>] ? get_cfcnfg+0x3a/0x180 > [ 200.752016] [<ffffffff821cf0b0>] ? lockdep_rtnl_is_held+0x10/0x20 > [ 200.752016] [<ffffffff825c41be>] caif_device_notify+0x2e/0x530 > [ 200.752016] [<ffffffff810d61b7>] notifier_call_chain+0x67/0x110 > [ 200.752016] [<ffffffff810d67c1>] raw_notifier_call_chain+0x11/0x20 > [ 200.752016] [<ffffffff821bae82>] call_netdevice_notifiers+0x32/0x60 > [ 200.752016] [<ffffffff821c2b26>] register_netdevice+0x196/0x300 > [ 200.752016] [<ffffffff821c2ca9>] register_netdev+0x19/0x30 > [ 200.752016] [<ffffffff81c1c67a>] loopback_net_init+0x4a/0xa0 > [ 200.752016] [<ffffffff821b5e62>] ops_init+0x42/0x180 > [ 200.752016] [<ffffffff821b600b>] setup_net+0x6b/0x100 > [ 200.752016] [<ffffffff821b6466>] copy_net_ns+0x86/0x110 > [ 200.752016] [<ffffffff810d5789>] create_new_namespaces+0xd9/0x190 > > Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com> > --- > > Hi Sasha, > > Do you have any chance to review and test this patch? > I'd like to get the net namespace handling this right this time ... > > Thanks, > Sjur Works for me. Tested-by: Sasha Levin <levinsasha928@gmail.com> -- 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
diff --git a/net/caif/caif_dev.c b/net/caif/caif_dev.c index 673728a..3197bc2 100644 --- a/net/caif/caif_dev.c +++ b/net/caif/caif_dev.c @@ -371,6 +371,13 @@ static int caif_device_notify(struct notifier_block *me, un struct cflayer *layer, *link_support; int head_room = 0; struct caif_device_entry_list *caifdevs; + int len; + + rcu_read_lock(); + len = rcu_dereference(dev_net(dev)->gen)->len; + rcu_read_unlock(); + if (caif_net_id > len) + return 0; cfg = get_cfcnfg(dev_net(dev)); caifdevs = caif_device_list(dev_net(dev));