OpenZFS 9457 - libzfs_import.c:add_config() has a memory leak

A memory leak occurs on lines 209 and 213 because the config is not
freed in the error case.  The interface to add_config() seems less than
ideal - it would be better if it copied any data necessary from the
config and the caller freed it.

Porting notes:
* This issue had already been resolved on Linux by adding the missing
  calls to nvlist_free().  But we'll adopt the upstream fix to keep
  the behavior of the code consistent.

Authored by: Sara Hartse <[email protected]>
Reviewed by: Matt Ahrens <[email protected]>
Reviewed by: Serapheim Dimitropoulos <[email protected]>
Reviewed by: Giuseppe Di Natale <[email protected]>
Reviewed by: George Melikov <[email protected]>
Approved by: Robert Mustacchi <[email protected]>
Ported-by: Brian Behlendorf <[email protected]>

OpenZFS-issue: https://illumos.org/issues/9457
OpenZFS-commit: https://github.com/openzfs/openzfs/commit/be86bb8a
Closes #7713

1 files changed, 6 insertions, 18 deletions

diff --git a/lib/libzfs/libzfs_import.c b/lib/libzfs/libzfs_import.c
index d2c7d98f9..634c076b8 100644
--- a/lib/libzfs/libzfs_import.c
+++ b/lib/libzfs/libzfs_import.c
@@ -33,7 +33,7 @@
  * ZFS label of each device.  If we successfully read the label, then we
  * organize the configuration information in the following hierarchy:
  *
- * 	pool guid -> toplevel vdev guid -> label txg
+ *	pool guid -> toplevel vdev guid -> label txg
  *
  * Duplicate entries matching this same tuple will be discarded.  Once we have
  * examined every device, we pick the best label txg config for each toplevel
@@ -729,14 +729,11 @@ add_config(libzfs_handle_t *hdl, pool_list_t *pl, const char *path,
 	    &state) == 0 &&
 	    (state == POOL_STATE_SPARE || state == POOL_STATE_L2CACHE) &&
 	    nvlist_lookup_uint64(config, ZPOOL_CONFIG_GUID, &vdev_guid) == 0) {
-		if ((ne = zfs_alloc(hdl, sizeof (name_entry_t))) == NULL) {
-			nvlist_free(config);
+		if ((ne = zfs_alloc(hdl, sizeof (name_entry_t))) == NULL)
 			return (-1);
-		}
 
 		if ((ne->ne_name = zfs_strdup(hdl, path)) == NULL) {
 			free(ne);
-			nvlist_free(config);
 			return (-1);
 		}
 		ne->ne_guid = vdev_guid;
@@ -744,7 +741,7 @@ add_config(libzfs_handle_t *hdl, pool_list_t *pl, const char *path,
 		ne->ne_num_labels = num_labels;
 		ne->ne_next = pl->names;
 		pl->names = ne;
-		nvlist_free(config);
+
 		return (0);
 	}
 
@@ -764,7 +761,6 @@ add_config(libzfs_handle_t *hdl, pool_list_t *pl, const char *path,
 	    &top_guid) != 0 ||
 	    nvlist_lookup_uint64(config, ZPOOL_CONFIG_POOL_TXG,
 	    &txg) != 0 || txg == 0) {
-		nvlist_free(config);
 		return (0);
 	}
 
@@ -779,7 +775,6 @@ add_config(libzfs_handle_t *hdl, pool_list_t *pl, const char *path,
 
 	if (pe == NULL) {
 		if ((pe = zfs_alloc(hdl, sizeof (pool_entry_t))) == NULL) {
-			nvlist_free(config);
 			return (-1);
 		}
 		pe->pe_guid = pool_guid;
@@ -798,7 +793,6 @@ add_config(libzfs_handle_t *hdl, pool_list_t *pl, const char *path,
 
 	if (ve == NULL) {
 		if ((ve = zfs_alloc(hdl, sizeof (vdev_entry_t))) == NULL) {
-			nvlist_free(config);
 			return (-1);
 		}
 		ve->ve_guid = top_guid;
@@ -818,15 +812,12 @@ add_config(libzfs_handle_t *hdl, pool_list_t *pl, const char *path,
 
 	if (ce == NULL) {
 		if ((ce = zfs_alloc(hdl, sizeof (config_entry_t))) == NULL) {
-			nvlist_free(config);
 			return (-1);
 		}
 		ce->ce_txg = txg;
-		ce->ce_config = config;
+		ce->ce_config = fnvlist_dup(config);
 		ce->ce_next = ve->ve_configs;
 		ve->ve_configs = ce;
-	} else {
-		nvlist_free(config);
 	}
 
 	/*
@@ -2101,9 +2092,7 @@ zpool_find_import_impl(libzfs_handle_t *hdl, importargs_t *iarg)
 				    ZPOOL_CONFIG_POOL_GUID, &this_guid) == 0 &&
 				    iarg->guid == this_guid;
 			}
-			if (!matched) {
-				nvlist_free(config);
-			} else {
+			if (matched) {
 				/*
 				 * Verify all remaining entries can be opened
 				 * exclusively. This will prune all underlying
@@ -2121,10 +2110,9 @@ zpool_find_import_impl(libzfs_handle_t *hdl, importargs_t *iarg)
 					add_config(hdl, &pools,
 					    slice->rn_name, slice->rn_order,
 					    slice->rn_num_labels, config);
-				} else {
-					nvlist_free(config);
 				}
 			}
+			nvlist_free(config);
 		}
 		free(slice->rn_name);
 		free(slice);