From fe18da60500b8b8aa2621fb1de7132cb1f0aebcf Mon Sep 17 00:00:00 2001 From: Girish Moodalbail Date: Thu, 16 Nov 2017 23:16:17 -0800 Subject: 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 Signed-off-by: David S. Miller --- drivers/net/ipvlan/ipvlan_main.c | 104 +++++++++++++++++++++------------------ 1 file 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); -- cgit v1.2.3-59-g8ed1b