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 <sara.hartse@delphix.com>
Reviewed by: Matt Ahrens <matt@delphix.com>
Reviewed by: Serapheim Dimitropoulos <serapheim.dimitro@delphix.com>
Reviewed by: Giuseppe Di Natale <guss80@gmail.com>
Reviewed by: George Melikov <mail@gmelikov.ru>
Approved by: Robert Mustacchi <rm@joyent.com>
Ported-by: Brian Behlendorf <behlendorf1@llnl.gov>

OpenZFS-issue: https://illumos.org/issues/9457
OpenZFS-commit: https://github.com/openzfs/openzfs/commit/be86bb8a
Closes #7713
This commit is contained in:
sara hartse 2017-09-07 13:10:53 -07:00 committed by Brian Behlendorf
parent 802b1a7b3b
commit 473c976a0c
1 changed files with 6 additions and 18 deletions

View File

@ -33,7 +33,7 @@
* ZFS label of each device. If we successfully read the label, then we * ZFS label of each device. If we successfully read the label, then we
* organize the configuration information in the following hierarchy: * 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 * 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 * 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) == 0 &&
(state == POOL_STATE_SPARE || state == POOL_STATE_L2CACHE) && (state == POOL_STATE_SPARE || state == POOL_STATE_L2CACHE) &&
nvlist_lookup_uint64(config, ZPOOL_CONFIG_GUID, &vdev_guid) == 0) { nvlist_lookup_uint64(config, ZPOOL_CONFIG_GUID, &vdev_guid) == 0) {
if ((ne = zfs_alloc(hdl, sizeof (name_entry_t))) == NULL) { if ((ne = zfs_alloc(hdl, sizeof (name_entry_t))) == NULL)
nvlist_free(config);
return (-1); return (-1);
}
if ((ne->ne_name = zfs_strdup(hdl, path)) == NULL) { if ((ne->ne_name = zfs_strdup(hdl, path)) == NULL) {
free(ne); free(ne);
nvlist_free(config);
return (-1); return (-1);
} }
ne->ne_guid = vdev_guid; 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_num_labels = num_labels;
ne->ne_next = pl->names; ne->ne_next = pl->names;
pl->names = ne; pl->names = ne;
nvlist_free(config);
return (0); return (0);
} }
@ -764,7 +761,6 @@ add_config(libzfs_handle_t *hdl, pool_list_t *pl, const char *path,
&top_guid) != 0 || &top_guid) != 0 ||
nvlist_lookup_uint64(config, ZPOOL_CONFIG_POOL_TXG, nvlist_lookup_uint64(config, ZPOOL_CONFIG_POOL_TXG,
&txg) != 0 || txg == 0) { &txg) != 0 || txg == 0) {
nvlist_free(config);
return (0); return (0);
} }
@ -779,7 +775,6 @@ add_config(libzfs_handle_t *hdl, pool_list_t *pl, const char *path,
if (pe == NULL) { if (pe == NULL) {
if ((pe = zfs_alloc(hdl, sizeof (pool_entry_t))) == NULL) { if ((pe = zfs_alloc(hdl, sizeof (pool_entry_t))) == NULL) {
nvlist_free(config);
return (-1); return (-1);
} }
pe->pe_guid = pool_guid; 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 == NULL) {
if ((ve = zfs_alloc(hdl, sizeof (vdev_entry_t))) == NULL) { if ((ve = zfs_alloc(hdl, sizeof (vdev_entry_t))) == NULL) {
nvlist_free(config);
return (-1); return (-1);
} }
ve->ve_guid = top_guid; 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 == NULL) {
if ((ce = zfs_alloc(hdl, sizeof (config_entry_t))) == NULL) { if ((ce = zfs_alloc(hdl, sizeof (config_entry_t))) == NULL) {
nvlist_free(config);
return (-1); return (-1);
} }
ce->ce_txg = txg; ce->ce_txg = txg;
ce->ce_config = config; ce->ce_config = fnvlist_dup(config);
ce->ce_next = ve->ve_configs; ce->ce_next = ve->ve_configs;
ve->ve_configs = ce; 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 && ZPOOL_CONFIG_POOL_GUID, &this_guid) == 0 &&
iarg->guid == this_guid; iarg->guid == this_guid;
} }
if (!matched) { if (matched) {
nvlist_free(config);
} else {
/* /*
* Verify all remaining entries can be opened * Verify all remaining entries can be opened
* exclusively. This will prune all underlying * 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, add_config(hdl, &pools,
slice->rn_name, slice->rn_order, slice->rn_name, slice->rn_order,
slice->rn_num_labels, config); slice->rn_num_labels, config);
} else {
nvlist_free(config);
} }
} }
nvlist_free(config);
} }
free(slice->rn_name); free(slice->rn_name);
free(slice); free(slice);