7413 netstack leaks lead to blind netstack reuse

7414 iptun leaks netstacks
7415 ipnet_promisc_add() leaks netstacks in error paths
7416 ill_lookup_on_ifindex_global_instance() could leak netstack ids
Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
Reviewed by: Robert Mustacchi <rm@joyent.com>
Reviewed by: Sebastien Roy <sebastien.roy@delphix.com>
Approved by: Dan McDonald <danmcd@omniti.com>

6 files changed, 106 insertions, 20 deletions

diff --git a/usr/src/uts/common/inet/ip/ip_if.c b/usr/src/uts/common/inet/ip/ip_if.c
index 62d85b202b..b88dcae2d1 100644
--- a/usr/src/uts/common/inet/ip/ip_if.c
+++ b/usr/src/uts/common/inet/ip/ip_if.c
@@ -22,7 +22,7 @@
  * Copyright (c) 1991, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 1990 Mentat Inc.
  * Copyright (c) 2013 by Delphix. All rights reserved.
- * Copyright 2013 Joyent, Inc.
+ * Copyright (c) 2016, Joyent, Inc. All rights reserved.
  * Copyright (c) 2014, OmniTI Computer Consulting, Inc. All rights reserved.
  */
 
@@ -718,7 +718,7 @@ ill_dlur_copy_address(uchar_t *phys_src, uint_t phys_length,
  */
 mblk_t *
 ill_dlur_gen(uchar_t *addr, uint_t addr_length, t_uscalar_t sap,
-		t_scalar_t sap_length)
+    t_scalar_t sap_length)
 {
 	dl_unitdata_req_t *dlur;
 	mblk_t	*mp;
@@ -3855,15 +3855,18 @@ ill_lookup_on_ifindex_global_instance(uint_t index, boolean_t isv6)
 {
 	ip_stack_t	*ipst;
 	ill_t		*ill;
+	netstack_t	*ns;
 
-	ipst = netstack_find_by_stackid(GLOBAL_NETSTACKID)->netstack_ip;
-	if (ipst == NULL) {
+	ns = netstack_find_by_stackid(GLOBAL_NETSTACKID);
+
+	if ((ipst = ns->netstack_ip) == NULL) {
 		cmn_err(CE_WARN, "No ip_stack_t for zoneid zero!\n");
+		netstack_rele(ns);
 		return (NULL);
 	}
 
 	ill = ill_lookup_on_ifindex(index, isv6, ipst);
-	netstack_rele(ipst->ips_netstack);
+	netstack_rele(ns);
 	return (ill);
 }
 
@@ -10828,7 +10831,7 @@ ip_sioctl_mtu(ipif_t *ipif, sin_t *sin, queue_t *q, mblk_t *mp,
 /* ARGSUSED */
 int
 ip_sioctl_get_mtu(ipif_t *ipif, sin_t *sin, queue_t *q, mblk_t *mp,
-	ip_ioctl_cmd_t *ipip, void *if_req)
+    ip_ioctl_cmd_t *ipip, void *if_req)
 {
 	struct ifreq	*ifr;
 	struct lifreq	*lifr;
@@ -10854,7 +10857,7 @@ ip_sioctl_get_mtu(ipif_t *ipif, sin_t *sin, queue_t *q, mblk_t *mp,
 /* ARGSUSED2 */
 int
 ip_sioctl_brdaddr(ipif_t *ipif, sin_t *sin, queue_t *q, mblk_t *mp,
-	ip_ioctl_cmd_t *ipip, void *if_req)
+    ip_ioctl_cmd_t *ipip, void *if_req)
 {
 	ipaddr_t addr;
 	ire_t	*ire;
@@ -15590,7 +15593,7 @@ ip_select_source_v4(ill_t *ill, ipaddr_t setsrc, ipaddr_t dst,
 /* ARGSUSED */
 int
 if_unitsel_restart(ipif_t *ipif, sin_t *dummy_sin, queue_t *q, mblk_t *mp,
-	ip_ioctl_cmd_t *ipip, void *dummy_ifreq)
+    ip_ioctl_cmd_t *ipip, void *dummy_ifreq)
 {
 	/*
 	 * ill_phyint_reinit merged the v4 and v6 into a single
@@ -16247,7 +16250,7 @@ ill_ptpaddr_cnt(const ill_t *ill)
 /* ARGSUSED */
 int
 ip_sioctl_get_lifusesrc(ipif_t *ipif, sin_t *sin, queue_t *q, mblk_t *mp,
-	ip_ioctl_cmd_t *ipip, void *ifreq)
+    ip_ioctl_cmd_t *ipip, void *ifreq)
 {
 	struct lifreq	*lifr = ifreq;
 
diff --git a/usr/src/uts/common/inet/ipnet/ipnet.c b/usr/src/uts/common/inet/ipnet/ipnet.c
index 75c30120f6..2f53a48d80 100644
--- a/usr/src/uts/common/inet/ipnet/ipnet.c
+++ b/usr/src/uts/common/inet/ipnet/ipnet.c
@@ -25,6 +25,10 @@
  */
 
 /*
+ * Copyright (c) 2016, Joyent, Inc. All rights reserved.
+ */
+
+/*
  * The ipnet device defined here provides access to packets at the IP layer. To
  * provide access to packets at this layer it registers a callback function in
  * the ip module and when there are open instances of the device ip will pass
@@ -2181,14 +2185,15 @@ ipnet_promisc_add(void *handle, uint_t how, void *data, uintptr_t *mhandle,
 	int		error;
 
 	ifp = (ipnetif_t *)handle;
-	ns = netstack_find_by_zoneid(ifp->if_zoneid);
 
-	if ((how == DL_PROMISC_PHYS) || (how == DL_PROMISC_MULTI)) {
-		error = ipnet_join_allmulti(ifp, ns->netstack_ipnet);
-		if (error != 0)
-			return (error);
-	} else {
+	if (how != DL_PROMISC_PHYS && how != DL_PROMISC_MULTI)
 		return (EINVAL);
+
+	ns = netstack_find_by_zoneid(ifp->if_zoneid);
+
+	if ((error = ipnet_join_allmulti(ifp, ns->netstack_ipnet)) != 0) {
+		netstack_rele(ns);
+		return (error);
 	}
 
 	ipnet = kmem_zalloc(sizeof (*ipnet), KM_SLEEP);
diff --git a/usr/src/uts/common/inet/iptun/iptun.c b/usr/src/uts/common/inet/iptun/iptun.c
index c933efb470..fb4402dc17 100644
--- a/usr/src/uts/common/inet/iptun/iptun.c
+++ b/usr/src/uts/common/inet/iptun/iptun.c
@@ -20,6 +20,7 @@
  */
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, Joyent, Inc. All rights reserved.
  */
 
 /*
@@ -1355,6 +1356,7 @@ iptun_free(iptun_t *iptun)
 		iptun->iptun_connp = NULL;
 	}
 
+	netstack_rele(iptun->iptun_ns);
 	kmem_cache_free(iptun_cache, iptun);
 	atomic_dec_32(&iptun_tunnelcount);
 }
diff --git a/usr/src/uts/common/os/netstack.c b/usr/src/uts/common/os/netstack.c
index 5c556bffef..1bed9d7406 100644
--- a/usr/src/uts/common/os/netstack.c
+++ b/usr/src/uts/common/os/netstack.c
@@ -22,7 +22,7 @@
 /*
  * Copyright 2009 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
- * Copyright (c) 2013, Joyent, Inc.  All rights reserved.
+ * Copyright (c) 2016, Joyent, Inc.  All rights reserved.
  */
 
 #include <sys/param.h>
@@ -340,7 +340,7 @@ netstack_zone_create(zoneid_t zoneid)
 			/*
 			 * Should never find a pre-existing exclusive stack
 			 */
-			ASSERT(stackid == GLOBAL_NETSTACKID);
+			VERIFY(stackid == GLOBAL_NETSTACKID);
 			kmem_free(ns, sizeof (netstack_t));
 			ns = *nsp;
 			mutex_enter(&ns->netstack_lock);
@@ -1041,6 +1041,26 @@ netstack_find_by_stackid(netstackid_t stackid)
 	return (NULL);
 }
 
+boolean_t
+netstack_inuse_by_stackid(netstackid_t stackid)
+{
+	netstack_t *ns;
+	boolean_t rval = B_FALSE;
+
+	mutex_enter(&netstack_g_lock);
+
+	for (ns = netstack_head; ns != NULL; ns = ns->netstack_next) {
+		if (ns->netstack_stackid == stackid) {
+			rval = B_TRUE;
+			break;
+		}
+	}
+
+	mutex_exit(&netstack_g_lock);
+
+	return (rval);
+}
+
 void
 netstack_rele(netstack_t *ns)
 {
diff --git a/usr/src/uts/common/os/zone.c b/usr/src/uts/common/os/zone.c
index 1028f9ce3f..20d7311af7 100644
--- a/usr/src/uts/common/os/zone.c
+++ b/usr/src/uts/common/os/zone.c
@@ -4271,7 +4271,8 @@ out:
 }
 
 int
-zone_create_error(int er_error, int er_ext, int *er_out) {
+zone_create_error(int er_error, int er_ext, int *er_out)
+{
 	if (er_out != NULL) {
 		if (copyout(&er_ext, er_out, sizeof (int))) {
 			return (set_errno(EFAULT));
@@ -4367,7 +4368,7 @@ zone_create(const char *zone_name, const char *zone_root,
 	nvlist_t *rctls = NULL;
 	proc_t *pp = curproc;
 	zone_t *zone, *ztmp;
-	zoneid_t zoneid;
+	zoneid_t zoneid, start = GLOBAL_ZONEID;
 	int error;
 	int error2 = 0;
 	char *str;
@@ -4381,9 +4382,58 @@ zone_create(const char *zone_name, const char *zone_root,
 	if (PTOU(pp)->u_rdir != NULL && PTOU(pp)->u_rdir != rootdir)
 		return (zone_create_error(ENOTSUP, ZE_CHROOTED,
 		    extended_error));
+	/*
+	 * As the first step of zone creation, we want to allocate a zoneid.
+	 * This allocation is complicated by the fact that netstacks use the
+	 * zoneid to determine their stackid, but netstacks themselves are
+	 * freed asynchronously with respect to zone destruction.  This means
+	 * that a netstack reference leak (or in principle, an extraordinarily
+	 * long netstack reference hold) could result in a zoneid being
+	 * allocated that in fact corresponds to a stackid from an active
+	 * (referenced) netstack -- unleashing all sorts of havoc when that
+	 * netstack is actually (re)used.  (In the abstract, we might wish a
+	 * zoneid to not be deallocated until its last referencing netstack
+	 * has been released, but netstacks lack a backpointer into their
+	 * referencing zone -- and changing them to have such a pointer would
+	 * be substantial, to put it euphemistically.)  To avoid this, we
+	 * detect this condition on allocation: if we have allocated a zoneid
+	 * that corresponds to a netstack that's still in use, we warn about
+	 * it (as it is much more likely to be a reference leak than an actual
+	 * netstack reference), free it, and allocate another.  That these
+	 * identifers are allocated out of an ID space assures that we won't
+	 * see the identifier we just allocated.
+	 */
+	for (;;) {
+		zoneid = id_alloc(zoneid_space);
+
+		if (!netstack_inuse_by_stackid(zoneid_to_netstackid(zoneid)))
+			break;
+
+		id_free(zoneid_space, zoneid);
+
+		if (start == GLOBAL_ZONEID) {
+			start = zoneid;
+		} else if (zoneid == start) {
+			/*
+			 * We have managed to iterate over the entire available
+			 * zoneid space -- there are no identifiers available,
+			 * presumably due to some number of leaked netstack
+			 * references.  While it's in principle possible for us
+			 * to continue to try, it seems wiser to give up at
+			 * this point to warn and fail explicitly with a
+			 * distinctive error.
+			 */
+			cmn_err(CE_WARN, "zone_create() failed: all available "
+			    "zone IDs have netstacks still in use");
+			return (set_errno(ENFILE));
+		}
+
+		cmn_err(CE_WARN, "unable to reuse zone ID %d; "
+		    "netstack still in use", zoneid);
+	}
 
 	zone = kmem_zalloc(sizeof (zone_t), KM_SLEEP);
-	zoneid = zone->zone_id = id_alloc(zoneid_space);
+	zone->zone_id = zoneid;
 	zone->zone_status = ZONE_IS_UNINITIALIZED;
 	zone->zone_pool = pool_default;
 	zone->zone_pool_mod = gethrtime();
diff --git a/usr/src/uts/common/sys/netstack.h b/usr/src/uts/common/sys/netstack.h
index 2c77e1be96..edf703f2ef 100644
--- a/usr/src/uts/common/sys/netstack.h
+++ b/usr/src/uts/common/sys/netstack.h
@@ -23,6 +23,11 @@
  * Copyright 2009 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
+
+/*
+ * Copyright (c) 2016, Joyent, Inc. All rights reserved.
+ */
+
 #ifndef _SYS_NETSTACK_H
 #define	_SYS_NETSTACK_H
 
@@ -233,6 +238,7 @@ extern void netstack_rele(netstack_t *);
 extern netstack_t *netstack_find_by_cred(const cred_t *);
 extern netstack_t *netstack_find_by_stackid(netstackid_t);
 extern netstack_t *netstack_find_by_zoneid(zoneid_t);
+extern boolean_t netstack_inuse_by_stackid(netstackid_t stackid);
 
 extern zoneid_t netstackid_to_zoneid(netstackid_t);
 extern zoneid_t netstack_get_zoneid(netstack_t *);