6433264 crash when adding spare: nvlist_lookup_string(cnv, "path", &path) == 0

6433406 zfs_open() can leak memory on failure
6433408 namespace_reload() can leak memory on allocation failure
6433679 zpool_refresh_stats() has poor error semantics
6433680 changelist_gather() ignores libuutil errors

9 files changed, 138 insertions, 64 deletions

diff --git a/usr/src/cmd/zpool/zpool_main.c b/usr/src/cmd/zpool/zpool_main.c
index c963776a9f..9a82a148ea 100644
--- a/usr/src/cmd/zpool/zpool_main.c
+++ b/usr/src/cmd/zpool/zpool_main.c
@@ -1472,11 +1472,15 @@ static int
 refresh_iostat(zpool_handle_t *zhp, void *data)
 {
 	iostat_cbdata_t *cb = data;
+	boolean_t missing;
 
 	/*
 	 * If the pool has disappeared, remove it from the list and continue.
 	 */
-	if (zpool_refresh_stats(zhp) != 0)
+	if (zpool_refresh_stats(zhp, &missing) != 0)
+		return (-1);
+
+	if (missing)
 		pool_list_remove(cb->cb_list, zhp);
 
 	return (0);
diff --git a/usr/src/cmd/zpool/zpool_vdev.c b/usr/src/cmd/zpool/zpool_vdev.c
index fa106dffb9..4399e4c99b 100644
--- a/usr/src/cmd/zpool/zpool_vdev.c
+++ b/usr/src/cmd/zpool/zpool_vdev.c
@@ -590,6 +590,28 @@ get_replication(nvlist_t *nvroot, boolean_t fatal)
 
 				verify(nvlist_lookup_string(cnv,
 				    ZPOOL_CONFIG_TYPE, &childtype) == 0);
+
+				/*
+				 * If this is a a replacing or spare vdev, then
+				 * get the real first child of the vdev.
+				 */
+				if (strcmp(childtype,
+				    VDEV_TYPE_REPLACING) == 0 ||
+				    strcmp(childtype, VDEV_TYPE_SPARE) == 0) {
+					nvlist_t **rchild;
+					uint_t rchildren;
+
+					verify(nvlist_lookup_nvlist_array(cnv,
+					    ZPOOL_CONFIG_CHILDREN, &rchild,
+					    &rchildren) == 0);
+					assert(rchildren == 2);
+					cnv = rchild[0];
+
+					verify(nvlist_lookup_string(cnv,
+					    ZPOOL_CONFIG_TYPE,
+					    &childtype) == 0);
+				}
+
 				verify(nvlist_lookup_string(cnv,
 				    ZPOOL_CONFIG_PATH, &path) == 0);
 
diff --git a/usr/src/lib/libzfs/common/libzfs.h b/usr/src/lib/libzfs/common/libzfs.h
index bf4b2874ad..410649bdcb 100644
--- a/usr/src/lib/libzfs/common/libzfs.h
+++ b/usr/src/lib/libzfs/common/libzfs.h
@@ -197,7 +197,7 @@ extern zpool_status_t zpool_import_status(nvlist_t *, char **);
  * Statistics and configuration functions.
  */
 extern nvlist_t *zpool_get_config(zpool_handle_t *, nvlist_t **);
-extern int zpool_refresh_stats(zpool_handle_t *);
+extern int zpool_refresh_stats(zpool_handle_t *, boolean_t *);
 extern int zpool_get_errlog(zpool_handle_t *, nvlist_t ***, size_t *);
 
 #define	ZPOOL_ERR_DATASET	"dataset"
diff --git a/usr/src/lib/libzfs/common/libzfs_changelist.c b/usr/src/lib/libzfs/common/libzfs_changelist.c
index 04270dfe51..ed1b291ef3 100644
--- a/usr/src/lib/libzfs/common/libzfs_changelist.c
+++ b/usr/src/lib/libzfs/common/libzfs_changelist.c
@@ -316,21 +316,24 @@ changelist_free(prop_changelist_t *clp)
 	prop_changenode_t *cn;
 	uu_list_walk_t *walk;
 
-	verify((walk = uu_list_walk_start(clp->cl_list,
-	    UU_WALK_ROBUST)) != NULL);
+	if (clp->cl_list) {
+		verify((walk = uu_list_walk_start(clp->cl_list,
+		    UU_WALK_ROBUST)) != NULL);
 
-	while ((cn = uu_list_walk_next(walk)) != NULL) {
+		while ((cn = uu_list_walk_next(walk)) != NULL) {
 
-		uu_list_remove(clp->cl_list, cn);
+			uu_list_remove(clp->cl_list, cn);
 
-		zfs_close(cn->cn_handle);
-		free(cn);
-	}
+			zfs_close(cn->cn_handle);
+			free(cn);
+		}
 
-	uu_list_walk_end(walk);
+		uu_list_walk_end(walk);
 
-	uu_list_destroy(clp->cl_list);
-	uu_list_pool_destroy(clp->cl_pool);
+		uu_list_destroy(clp->cl_list);
+	}
+	if (clp->cl_pool)
+		uu_list_pool_destroy(clp->cl_pool);
 
 	free(clp);
 }
@@ -421,11 +424,23 @@ changelist_gather(zfs_handle_t *zhp, zfs_prop_t prop, int flags)
 	    sizeof (prop_changenode_t),
 	    offsetof(prop_changenode_t, cn_listnode),
 	    NULL, 0);
-	assert(clp->cl_pool != NULL);
+	if (clp->cl_pool == NULL) {
+		assert(uu_error() == UU_ERROR_NO_MEMORY);
+		(void) zfs_error(zhp->zfs_hdl, EZFS_NOMEM, "internal error");
+		changelist_free(clp);
+		return (NULL);
+	}
 
 	clp->cl_list = uu_list_create(clp->cl_pool, NULL, 0);
 	clp->cl_flags = flags;
 
+	if (clp->cl_list == NULL) {
+		assert(uu_error() == UU_ERROR_NO_MEMORY);
+		(void) zfs_error(zhp->zfs_hdl, EZFS_NOMEM, "internal error");
+		changelist_free(clp);
+		return (NULL);
+	}
+
 	/*
 	 * If this is a rename or the 'zoned' property, we pretend we're
 	 * changing the mountpoint and flag it so we can catch all children in
diff --git a/usr/src/lib/libzfs/common/libzfs_config.c b/usr/src/lib/libzfs/common/libzfs_config.c
index be691f0ced..53f6bc2df4 100644
--- a/usr/src/lib/libzfs/common/libzfs_config.c
+++ b/usr/src/lib/libzfs/common/libzfs_config.c
@@ -173,8 +173,10 @@ namespace_reload(libzfs_handle_t *hdl)
 	}
 
 	if (nvlist_unpack((void *)(uintptr_t)zc.zc_config_dst,
-	    zc.zc_config_dst_size, &config, 0) != 0)
+	    zc.zc_config_dst_size, &config, 0) != 0) {
+		free((void *)(uintptr_t)zc.zc_config_dst);
 		return (no_memory(hdl));
+	}
 
 	free((void *)(uintptr_t)zc.zc_config_dst);
 
@@ -246,12 +248,13 @@ zpool_get_config(zpool_handle_t *zhp, nvlist_t **oldconfig)
  * been destroyed.
  */
 int
-zpool_refresh_stats(zpool_handle_t *zhp)
+zpool_refresh_stats(zpool_handle_t *zhp, boolean_t *missing)
 {
 	zfs_cmd_t zc = { 0 };
 	int error;
 	nvlist_t *config;
 
+	*missing = B_FALSE;
 	(void) strcpy(zc.zc_name, zhp->zpool_name);
 
 	if (zhp->zpool_config_size == 0)
@@ -268,7 +271,7 @@ zpool_refresh_stats(zpool_handle_t *zhp)
 			/*
 			 * The real error is returned in the zc_cookie field.
 			 */
-			error = errno = zc.zc_cookie;
+			error = zc.zc_cookie;
 			break;
 		}
 
@@ -280,7 +283,10 @@ zpool_refresh_stats(zpool_handle_t *zhp)
 				return (-1);
 		} else {
 			free((void *)(uintptr_t)zc.zc_config_dst);
-			return (-1);
+			if (errno == ENOENT || errno == EINVAL)
+				*missing = B_TRUE;
+			zhp->zpool_state = POOL_STATE_UNAVAIL;
+			return (0);
 		}
 	}
 
@@ -293,8 +299,10 @@ zpool_refresh_stats(zpool_handle_t *zhp)
 	zhp->zpool_config_size = zc.zc_config_dst_size;
 	free((void *)(uintptr_t)zc.zc_config_dst);
 
-	if (set_pool_health(config) != 0)
+	if (set_pool_health(config) != 0) {
+		nvlist_free(config);
 		return (no_memory(zhp->zpool_hdl));
+	}
 
 	if (zhp->zpool_config != NULL) {
 		uint64_t oldtxg, newtxg;
@@ -316,8 +324,12 @@ zpool_refresh_stats(zpool_handle_t *zhp)
 	}
 
 	zhp->zpool_config = config;
+	if (error)
+		zhp->zpool_state = POOL_STATE_UNAVAIL;
+	else
+		zhp->zpool_state = POOL_STATE_ACTIVE;
 
-	return (error ? -1 : 0);
+	return (0);
 }
 
 /*
@@ -336,7 +348,10 @@ zpool_iter(libzfs_handle_t *hdl, zpool_iter_f func, void *data)
 	for (cn = uu_avl_first(hdl->libzfs_ns_avl); cn != NULL;
 	    cn = uu_avl_next(hdl->libzfs_ns_avl, cn)) {
 
-		if ((zhp = zpool_open_silent(hdl, cn->cn_name)) == NULL)
+		if (zpool_open_silent(hdl, cn->cn_name, &zhp) != 0)
+			return (-1);
+
+		if (zhp == NULL)
 			continue;
 
 		if ((ret = func(zhp, data)) != 0)
diff --git a/usr/src/lib/libzfs/common/libzfs_dataset.c b/usr/src/lib/libzfs/common/libzfs_dataset.c
index 14ba6112ed..9ff3a083be 100644
--- a/usr/src/lib/libzfs/common/libzfs_dataset.c
+++ b/usr/src/lib/libzfs/common/libzfs_dataset.c
@@ -351,7 +351,7 @@ zfs_open(libzfs_handle_t *hdl, const char *path, int types)
 
 	if (!(types & zhp->zfs_type)) {
 		(void) zfs_error(hdl, EZFS_BADTYPE, errbuf);
-		free(zhp);
+		zfs_close(zhp);
 		return (NULL);
 	}
 
diff --git a/usr/src/lib/libzfs/common/libzfs_impl.h b/usr/src/lib/libzfs/common/libzfs_impl.h
index 2c5e890767..9341a97546 100644
--- a/usr/src/lib/libzfs/common/libzfs_impl.h
+++ b/usr/src/lib/libzfs/common/libzfs_impl.h
@@ -105,7 +105,7 @@ void remove_mountpoint(zfs_handle_t *);
 zfs_handle_t *make_dataset_handle(libzfs_handle_t *, const char *);
 int set_pool_health(nvlist_t *);
 
-zpool_handle_t *zpool_open_silent(libzfs_handle_t *, const char *);
+int zpool_open_silent(libzfs_handle_t *, const char *, zpool_handle_t **);
 
 int zvol_create_link(libzfs_handle_t *, const char *);
 int zvol_remove_link(libzfs_handle_t *, const char *);
diff --git a/usr/src/lib/libzfs/common/libzfs_import.c b/usr/src/lib/libzfs/common/libzfs_import.c
index ef34419146..c11e65946b 100644
--- a/usr/src/lib/libzfs/common/libzfs_import.c
+++ b/usr/src/lib/libzfs/common/libzfs_import.c
@@ -337,21 +337,28 @@ add_config(libzfs_handle_t *hdl, pool_list_t *pl, const char *path,
 /*
  * Returns true if the named pool matches the given GUID.
  */
-static boolean_t
-pool_active(libzfs_handle_t *hdl, const char *name, uint64_t guid)
+static int
+pool_active(libzfs_handle_t *hdl, const char *name, uint64_t guid,
+    boolean_t *isactive)
 {
 	zpool_handle_t *zhp;
 	uint64_t theguid;
 
-	if ((zhp = zpool_open_silent(hdl, name)) == NULL)
-		return (B_FALSE);
+	if (zpool_open_silent(hdl, name, &zhp) != 0)
+		return (-1);
+
+	if (zhp == NULL) {
+		*isactive = B_FALSE;
+		return (0);
+	}
 
 	verify(nvlist_lookup_uint64(zhp->zpool_config, ZPOOL_CONFIG_POOL_GUID,
 	    &theguid) == 0);
 
 	zpool_close(zhp);
 
-	return (theguid == guid);
+	*isactive = (theguid == guid);
+	return (0);
 }
 
 /*
@@ -381,6 +388,7 @@ get_configs(libzfs_handle_t *hdl, pool_list_t *pl)
 	uint_t children = 0;
 	nvlist_t **child = NULL;
 	uint_t c;
+	boolean_t isactive;
 
 	if (nvlist_alloc(&ret, 0, 0) != 0)
 		goto nomem;
@@ -555,7 +563,10 @@ get_configs(libzfs_handle_t *hdl, pool_list_t *pl)
 		verify(nvlist_lookup_uint64(config, ZPOOL_CONFIG_POOL_GUID,
 		    &guid) == 0);
 
-		if (pool_active(hdl, name, guid)) {
+		if (pool_active(hdl, name, guid, &isactive) != 0)
+			goto error;
+
+		if (!isactive) {
 			nvlist_free(config);
 			config = NULL;
 			continue;
@@ -641,14 +652,11 @@ get_configs(libzfs_handle_t *hdl, pool_list_t *pl)
 nomem:
 	(void) no_memory(hdl);
 error:
-	if (config)
-		nvlist_free(config);
-	if (ret)
-		nvlist_free(ret);
+	nvlist_free(config);
+	nvlist_free(ret);
 	for (c = 0; c < children; c++)
 		nvlist_free(child[c]);
-	if (child)
-		free(child);
+	free(child);
 
 	return (NULL);
 }
@@ -896,6 +904,7 @@ zpool_in_use(libzfs_handle_t *hdl, int fd, pool_state_t *state, char **namestr,
 	nvlist_t *pool_config;
 	uint64_t stateval;
 	spare_cbdata_t cb = { 0 };
+	boolean_t isactive;
 
 	*inuse = B_FALSE;
 
@@ -932,7 +941,12 @@ zpool_in_use(libzfs_handle_t *hdl, int fd, pool_state_t *state, char **namestr,
 		 * active pool that was disconnected without being explicitly
 		 * exported.
 		 */
-		if (pool_active(hdl, name, guid)) {
+		if (pool_active(hdl, name, guid, &isactive) != 0) {
+			nvlist_free(config);
+			return (-1);
+		}
+
+		if (isactive) {
 			/*
 			 * Because the device may have been removed while
 			 * offlined, we only report it as active if the vdev is
diff --git a/usr/src/lib/libzfs/common/libzfs_pool.c b/usr/src/lib/libzfs/common/libzfs_pool.c
index 37c82015b9..dc25ea6d17 100644
--- a/usr/src/lib/libzfs/common/libzfs_pool.c
+++ b/usr/src/lib/libzfs/common/libzfs_pool.c
@@ -154,6 +154,7 @@ zpool_handle_t *
 zpool_open_canfail(libzfs_handle_t *hdl, const char *pool)
 {
 	zpool_handle_t *zhp;
+	boolean_t missing;
 
 	/*
 	 * Make sure the pool name is valid.
@@ -171,20 +172,19 @@ zpool_open_canfail(libzfs_handle_t *hdl, const char *pool)
 	zhp->zpool_hdl = hdl;
 	(void) strlcpy(zhp->zpool_name, pool, sizeof (zhp->zpool_name));
 
-	if (zpool_refresh_stats(zhp) != 0) {
-		if (errno == ENOENT || errno == EINVAL) {
-			zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
-			    "no such pool"));
-			(void) zfs_error(hdl, EZFS_NOENT,
-			    dgettext(TEXT_DOMAIN, "cannot open '%s'"),
-			    pool);
-			free(zhp);
-			return (NULL);
-		} else {
-			zhp->zpool_state = POOL_STATE_UNAVAIL;
-		}
-	} else {
-		zhp->zpool_state = POOL_STATE_ACTIVE;
+	if (zpool_refresh_stats(zhp, &missing) != 0) {
+		zpool_close(zhp);
+		return (NULL);
+	}
+
+	if (missing) {
+		zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+		    "no such pool"));
+		(void) zfs_error(hdl, EZFS_NOENT,
+		    dgettext(TEXT_DOMAIN, "cannot open '%s'"),
+		    pool);
+		zpool_close(zhp);
+		return (NULL);
 	}
 
 	return (zhp);
@@ -194,29 +194,31 @@ zpool_open_canfail(libzfs_handle_t *hdl, const char *pool)
  * Like the above, but silent on error.  Used when iterating over pools (because
  * the configuration cache may be out of date).
  */
-zpool_handle_t *
-zpool_open_silent(libzfs_handle_t *hdl, const char *pool)
+int
+zpool_open_silent(libzfs_handle_t *hdl, const char *pool, zpool_handle_t **ret)
 {
 	zpool_handle_t *zhp;
+	boolean_t missing;
 
-	if ((zhp = calloc(sizeof (zpool_handle_t), 1)) == NULL)
-		return (NULL);
+	if ((zhp = zfs_alloc(hdl, sizeof (zpool_handle_t))) == NULL)
+		return (-1);
 
 	zhp->zpool_hdl = hdl;
 	(void) strlcpy(zhp->zpool_name, pool, sizeof (zhp->zpool_name));
 
-	if (zpool_refresh_stats(zhp) != 0) {
-		if (errno == ENOENT || errno == EINVAL) {
-			free(zhp);
-			return (NULL);
-		} else {
-			zhp->zpool_state = POOL_STATE_UNAVAIL;
-		}
-	} else {
-		zhp->zpool_state = POOL_STATE_ACTIVE;
+	if (zpool_refresh_stats(zhp, &missing) != 0) {
+		zpool_close(zhp);
+		return (-1);
 	}
 
-	return (zhp);
+	if (missing) {
+		zpool_close(zhp);
+		*ret = NULL;
+		return (0);
+	}
+
+	*ret = zhp;
+	return (0);
 }
 
 /*
@@ -708,7 +710,9 @@ zpool_import(libzfs_handle_t *hdl, nvlist_t *config, const char *newname,
 		/*
 		 * This should never fail, but play it safe anyway.
 		 */
-		if ((zhp = zpool_open_silent(hdl, thename)) != NULL) {
+		if (zpool_open_silent(hdl, thename, &zhp) != 0) {
+			ret = -1;
+		} else if (zhp != NULL) {
 			ret = zpool_create_zvol_links(zhp);
 			zpool_close(zhp);
 		}