netdev - re: netdevice wanrouter: Convert directly reference of netdev->priv

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 3 Dec 2012 12:04:05 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	wangchen@...fujitsu.com
Cc:	netdev@...r.kernel.org
Subject: re: netdevice wanrouter: Convert directly reference of netdev->priv

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.

@@ -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