ipvlan: NULL pointer dereference panic in ipvlan_port_destroy

When call to register_netdevice() (called from ipvlan_link_new()) fails,
we call ipvlan_uninit() (through ndo_uninit()) to destroy the ipvlan
port. After returning unsuccessfully from register_netdevice() we go
ahead and call ipvlan_port_destroy() again which causes NULL pointer
dereference panic. Fix the issue by making ipvlan_init() and
ipvlan_uninit() call symmetric.

The ipvlan port will now be created inside ipvlan_init() and will be
destroyed in ipvlan_uninit().

Fixes: 2ad7bf363841 (ipvlan: Initial check-in of the IPVLAN driver)
Signed-off-by: Girish Moodalbail <girish.moodalbail@oracle.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

1 files changed, 55 insertions, 49 deletions

diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index a266aa435d4d..30cb803e2fe5 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -107,16 +107,6 @@ static int ipvlan_port_create(struct net_device *dev)
         struct ipvl_port *port;
         int err, idx;
 
-        if (dev->type != ARPHRD_ETHER || dev->flags & IFF_LOOPBACK) {
-                netdev_err(dev, "Master is either lo or non-ether device\n");
-                return -EINVAL;
-        }
-
-        if (netdev_is_rx_handler_busy(dev)) {
-                netdev_err(dev, "Device is already in use.\n");
-                return -EBUSY;
-        }
-
         port = kzalloc(sizeof(struct ipvl_port), GFP_KERNEL);
         if (!port)
                 return -ENOMEM;
@@ -179,8 +169,9 @@ static void ipvlan_port_destroy(struct net_device *dev)
 static int ipvlan_init(struct net_device *dev)
 {
         struct ipvl_dev *ipvlan = netdev_priv(dev);
-        const struct net_device *phy_dev = ipvlan->phy_dev;
-        struct ipvl_port *port = ipvlan->port;
+        struct net_device *phy_dev = ipvlan->phy_dev;
+        struct ipvl_port *port;
+        int err;
 
         dev->state = (dev->state & ~IPVLAN_STATE_MASK) |
                      (phy_dev->state & IPVLAN_STATE_MASK);
@@ -196,18 +187,27 @@ static int ipvlan_init(struct net_device *dev)
         if (!ipvlan->pcpu_stats)
                 return -ENOMEM;
 
+        if (!netif_is_ipvlan_port(phy_dev)) {
+                err = ipvlan_port_create(phy_dev);
+                if (err < 0) {
+                        free_percpu(ipvlan->pcpu_stats);
+                        return err;
+                }
+        }
+        port = ipvlan_port_get_rtnl(phy_dev);
         port->count += 1;
-
         return 0;
 }
 
 static void ipvlan_uninit(struct net_device *dev)
 {
         struct ipvl_dev *ipvlan = netdev_priv(dev);
-        struct ipvl_port *port = ipvlan->port;
+        struct net_device *phy_dev = ipvlan->phy_dev;
+        struct ipvl_port *port;
 
         free_percpu(ipvlan->pcpu_stats);
 
+        port = ipvlan_port_get_rtnl(phy_dev);
         port->count -= 1;
         if (!port->count)
                 ipvlan_port_destroy(port->dev);
@@ -554,7 +554,6 @@ int ipvlan_link_new(struct net *src_net, struct net_device *dev,
         struct net_device *phy_dev;
         int err;
         u16 mode = IPVLAN_MODE_L3;
-        bool create = false;
 
         if (!tb[IFLA_LINK])
                 return -EINVAL;
@@ -568,28 +567,41 @@ int ipvlan_link_new(struct net *src_net, struct net_device *dev,
 
                 phy_dev = tmp->phy_dev;
         } else if (!netif_is_ipvlan_port(phy_dev)) {
-                err = ipvlan_port_create(phy_dev);
-                if (err < 0)
-                        return err;
-                create = true;
-        }
+                /* Exit early if the underlying link is invalid or busy */
+                if (phy_dev->type != ARPHRD_ETHER ||
+                    phy_dev->flags & IFF_LOOPBACK) {
+                        netdev_err(phy_dev,
+                                   "Master is either lo or non-ether device\n");
+                        return -EINVAL;
+                }
 
-        if (data && data[IFLA_IPVLAN_MODE])
-                mode = nla_get_u16(data[IFLA_IPVLAN_MODE]);
+                if (netdev_is_rx_handler_busy(phy_dev)) {
+                        netdev_err(phy_dev, "Device is already in use.\n");
+                        return -EBUSY;
+                }
+        }
 
-        port = ipvlan_port_get_rtnl(phy_dev);
         ipvlan->phy_dev = phy_dev;
         ipvlan->dev = dev;
-        ipvlan->port = port;
         ipvlan->sfeatures = IPVLAN_FEATURES;
         ipvlan_adjust_mtu(ipvlan, phy_dev);
         INIT_LIST_HEAD(&ipvlan->addrs);
 
-        /* Flags are per port and latest update overrides. User has
-         * to be consistent in setting it just like the mode attribute.
+        /* TODO Probably put random address here to be presented to the
+         * world but keep using the physical-dev address for the outgoing
+         * packets.
          */
-        if (data && data[IFLA_IPVLAN_FLAGS])
-                ipvlan->port->flags = nla_get_u16(data[IFLA_IPVLAN_FLAGS]);
+        memcpy(dev->dev_addr, phy_dev->dev_addr, ETH_ALEN);
+
+        dev->priv_flags |= IFF_IPVLAN_SLAVE;
+
+        err = register_netdevice(dev);
+        if (err < 0)
+                return err;
+
+        /* ipvlan_init() would have created the port, if required */
+        port = ipvlan_port_get_rtnl(phy_dev);
+        ipvlan->port = port;
 
         /* If the port-id base is at the MAX value, then wrap it around and
          * begin from 0x1 again. This may be due to a busy system where lots
@@ -609,31 +621,28 @@ int ipvlan_link_new(struct net *src_net, struct net_device *dev,
                 err = ida_simple_get(&port->ida, 0x1, port->dev_id_start,
                                      GFP_KERNEL);
         if (err < 0)
-                goto destroy_ipvlan_port;
+                goto unregister_netdev;
         dev->dev_id = err;
+
         /* Increment id-base to the next slot for the future assignment */
         port->dev_id_start = err + 1;
 
-        /* TODO Probably put random address here to be presented to the
-         * world but keep using the physical-dev address for the outgoing
-         * packets.
-         */
-        memcpy(dev->dev_addr, phy_dev->dev_addr, ETH_ALEN);
+        err = netdev_upper_dev_link(phy_dev, dev, extack);
+        if (err)
+                goto remove_ida;
 
-        dev->priv_flags |= IFF_IPVLAN_SLAVE;
+        /* Flags are per port and latest update overrides. User has
+         * to be consistent in setting it just like the mode attribute.
+         */
+        if (data && data[IFLA_IPVLAN_FLAGS])
+                port->flags = nla_get_u16(data[IFLA_IPVLAN_FLAGS]);
 
-        err = register_netdevice(dev);
-        if (err < 0)
-                goto remove_ida;
+        if (data && data[IFLA_IPVLAN_MODE])
+                mode = nla_get_u16(data[IFLA_IPVLAN_MODE]);
 
-        err = netdev_upper_dev_link(phy_dev, dev, extack);
-        if (err) {
-                goto unregister_netdev;
-        }
         err = ipvlan_set_port_mode(port, mode);
-        if (err) {
+        if (err)
                 goto unlink_netdev;
-        }
 
         list_add_tail_rcu(&ipvlan->pnode, &port->ipvlans);
         netif_stacked_transfer_operstate(phy_dev, dev);
@@ -641,13 +650,10 @@ int ipvlan_link_new(struct net *src_net, struct net_device *dev,
 
 unlink_netdev:
         netdev_upper_dev_unlink(phy_dev, dev);
-unregister_netdev:
-        unregister_netdevice(dev);
 remove_ida:
         ida_simple_remove(&port->ida, dev->dev_id);
-destroy_ipvlan_port:
-        if (create)
-                ipvlan_port_destroy(phy_dev);
+unregister_netdev:
+        unregister_netdevice(dev);
         return err;
 }
 EXPORT_SYMBOL_GPL(ipvlan_link_new);