lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 11 Dec 2012 19:58:15 -0500 From: Paul Gortmaker <paul.gortmaker@...driver.com> To: Dan Carpenter <dan.carpenter@...cle.com> Cc: wangchen@...fujitsu.com, netdev@...r.kernel.org, David Miller <davem@...emloft.net> Subject: Re: netdevice wanrouter: Convert directly reference of netdev->priv On Mon, Dec 3, 2012 at 4:04 AM, Dan Carpenter <dan.carpenter@...cle.com> wrote: > Hello Wang Chen, > > The patch 7be6065b39c3: "netdevice wanrouter: Convert directly > reference of netdev->priv" from Nov 20, 2008, leads to the following > Smatch warning: > net/wanrouter/wanmain.c:610 wanrouter_device_new_if() > error: potential NULL dereference 'dev'. > > This is an old patch from 2008. It removed the allocation in > wanrouter_device_new_if() so it looks like wanrouter has been completely > broken for four years. Hi Dan, Crap -- wishing I'd seen this earlier. There was an RFC patch for sending wanrouter to the bitbucket from Joe Perches, but aside from the obvious build failures in it that Dave found (and I fixed) there wasn't any real feedback (either positive or negative) to it: http://patchwork.ozlabs.org/patch/198830/ Knowing it has been non-functional for ~4 years is I think a key bit of information in justifying a removal, so folks like yourself and JuliaL don't waste cycles fixing/auditing dead code. But it will need to be 3.9 material now, it seems. Paul. -- > > @@ -589,10 +591,6 @@ static int wanrouter_device_new_if(struct wan_device *wandev, > err = -EPROTONOSUPPORT; > goto out; > } else { > - dev = kzalloc(sizeof(struct net_device), GFP_KERNEL); > - err = -ENOBUFS; > - if (dev == NULL) > - goto out; > err = wandev->new_if(wandev, dev, cnf); > > "dev" is still NULL after the call to ->new_if(). > > } > > Here is what the code looks like now: > > net/wanrouter/wanmain.c > 590 if (cnf->config_id == WANCONFIG_MPPP) { > 591 printk(KERN_INFO "%s: Wanpipe Mulit-Port PPP support has not been compiled in!\n", > 592 wandev->name); > 593 err = -EPROTONOSUPPORT; > 594 goto out; > 595 } else { > > We were supposed to allocate "dev" here. > > 596 err = wandev->new_if(wandev, dev, cnf); > 597 } > 598 > 599 if (!err) { > 600 /* Register network interface. This will invoke init() > 601 * function supplied by the driver. If device registered > 602 * successfully, add it to the interface list. > 603 */ > 604 > 605 #ifdef WANDEBUG > 606 printk(KERN_INFO "%s: registering interface %s...\n", > 607 wanrouter_modname, dev->name); > 608 #endif > 609 > 610 err = register_netdev(dev); > ^^^^^^^^^^^^^^^^^^^^ > > The kernel will always oops inside the call to register_netdev() because > "dev" is still NULL. > > I suspect we should just revert the patch? > > regards, > dan carpenter > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@...r.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists