From 078266a5aafa880521ea55488ef3d676f35e908e Mon Sep 17 00:00:00 2001 From: Marcel Telka Date: Mon, 12 Jan 2015 18:03:44 +0100 Subject: [PATCH] 5518 Memory leaks in libzfs import implementation Reviewed by: Dan Fields Reviewed by: Matthew Ahrens Reviewed by: Serghei Samsi Approved by: Dan McDonald --- usr/src/lib/libzfs/common/libzfs_import.c | 55 ++++++++++++++--------- usr/src/lib/libzfs/common/libzfs_pool.c | 25 ++++++----- 2 files changed, 47 insertions(+), 33 deletions(-) diff --git a/usr/src/lib/libzfs/common/libzfs_import.c b/usr/src/lib/libzfs/common/libzfs_import.c index ae33cd058e06..19f2fbc57e53 100644 --- a/usr/src/lib/libzfs/common/libzfs_import.c +++ b/usr/src/lib/libzfs/common/libzfs_import.c @@ -18,10 +18,11 @@ * * CDDL HEADER END */ + /* + * Copyright 2015 Nexenta Systems, Inc. All rights reserved. * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2013 by Delphix. All rights reserved. - * Copyright 2014 Nexenta Systems, Inc. All rights reserved. */ /* @@ -196,8 +197,10 @@ fix_paths(nvlist_t *nv, name_entry_t *names) if ((devid = get_devid(best->ne_name)) == NULL) { (void) nvlist_remove_all(nv, ZPOOL_CONFIG_DEVID); } else { - if (nvlist_add_string(nv, ZPOOL_CONFIG_DEVID, devid) != 0) + if (nvlist_add_string(nv, ZPOOL_CONFIG_DEVID, devid) != 0) { + devid_str_free(devid); return (-1); + } devid_str_free(devid); } @@ -663,8 +666,10 @@ get_configs(libzfs_handle_t *hdl, pool_list_t *pl, boolean_t active_ok) nvlist_add_uint64(holey, ZPOOL_CONFIG_ID, c) != 0 || nvlist_add_uint64(holey, - ZPOOL_CONFIG_GUID, 0ULL) != 0) + ZPOOL_CONFIG_GUID, 0ULL) != 0) { + nvlist_free(holey); goto nomem; + } child[c] = holey; } } @@ -1103,8 +1108,10 @@ zpool_clear_label(int fd) for (l = 0; l < VDEV_LABELS; l++) { if (pwrite64(fd, label, sizeof (vdev_label_t), - label_offset(size, l)) != sizeof (vdev_label_t)) + label_offset(size, l)) != sizeof (vdev_label_t)) { + free(label); return (-1); + } } free(label); @@ -1122,7 +1129,6 @@ static nvlist_t * zpool_find_import_impl(libzfs_handle_t *hdl, importargs_t *iarg) { int i, dirs = iarg->paths; - DIR *dirp = NULL; struct dirent64 *dp; char path[MAXPATHLEN]; char *end, **dir = iarg->path; @@ -1152,6 +1158,8 @@ zpool_find_import_impl(libzfs_handle_t *hdl, importargs_t *iarg) tpool_t *t; char *rdsk; int dfd; + boolean_t config_failed = B_FALSE; + DIR *dirp; /* use realpath to normalize the path */ if (realpath(dir[i], path) == 0) { @@ -1176,6 +1184,8 @@ zpool_find_import_impl(libzfs_handle_t *hdl, importargs_t *iarg) if ((dfd = open64(rdsk, O_RDONLY)) < 0 || (dirp = fdopendir(dfd)) == NULL) { + if (dfd >= 0) + (void) close(dfd); zfs_error_aux(hdl, strerror(errno)); (void) zfs_error_fmt(hdl, EZFS_BADPATH, dgettext(TEXT_DOMAIN, "cannot open '%s'"), @@ -1223,7 +1233,7 @@ zpool_find_import_impl(libzfs_handle_t *hdl, importargs_t *iarg) cookie = NULL; while ((slice = avl_destroy_nodes(&slice_cache, &cookie)) != NULL) { - if (slice->rn_config != NULL) { + if (slice->rn_config != NULL && !config_failed) { nvlist_t *config = slice->rn_config; boolean_t matched = B_TRUE; @@ -1244,13 +1254,16 @@ zpool_find_import_impl(libzfs_handle_t *hdl, importargs_t *iarg) } if (!matched) { nvlist_free(config); - config = NULL; - continue; + } else { + /* + * use the non-raw path for the config + */ + (void) strlcpy(end, slice->rn_name, + pathleft); + if (add_config(hdl, &pools, path, + config) != 0) + config_failed = B_TRUE; } - /* use the non-raw path for the config */ - (void) strlcpy(end, slice->rn_name, pathleft); - if (add_config(hdl, &pools, path, config) != 0) - goto error; } free(slice->rn_name); free(slice); @@ -1258,7 +1271,9 @@ zpool_find_import_impl(libzfs_handle_t *hdl, importargs_t *iarg) avl_destroy(&slice_cache); (void) closedir(dirp); - dirp = NULL; + + if (config_failed) + goto error; } ret = get_configs(hdl, &pools, iarg->can_be_active); @@ -1281,14 +1296,10 @@ zpool_find_import_impl(libzfs_handle_t *hdl, importargs_t *iarg) for (ne = pools.names; ne != NULL; ne = nenext) { nenext = ne->ne_next; - if (ne->ne_name) - free(ne->ne_name); + free(ne->ne_name); free(ne); } - if (dirp) - (void) closedir(dirp); - return (ret); } @@ -1646,9 +1657,9 @@ zpool_in_use(libzfs_handle_t *hdl, int fd, pool_state_t *state, char **namestr, cb.cb_type = ZPOOL_CONFIG_SPARES; if (zpool_iter(hdl, find_aux, &cb) == 1) { name = (char *)zpool_get_name(cb.cb_zhp); - ret = TRUE; + ret = B_TRUE; } else { - ret = FALSE; + ret = B_FALSE; } break; @@ -1662,9 +1673,9 @@ zpool_in_use(libzfs_handle_t *hdl, int fd, pool_state_t *state, char **namestr, cb.cb_type = ZPOOL_CONFIG_L2CACHE; if (zpool_iter(hdl, find_aux, &cb) == 1) { name = (char *)zpool_get_name(cb.cb_zhp); - ret = TRUE; + ret = B_TRUE; } else { - ret = FALSE; + ret = B_FALSE; } break; diff --git a/usr/src/lib/libzfs/common/libzfs_pool.c b/usr/src/lib/libzfs/common/libzfs_pool.c index 675286c756b7..4f8d0b614888 100644 --- a/usr/src/lib/libzfs/common/libzfs_pool.c +++ b/usr/src/lib/libzfs/common/libzfs_pool.c @@ -20,8 +20,8 @@ */ /* + * Copyright 2015 Nexenta Systems, Inc. All rights reserved. * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved. - * Copyright 2011 Nexenta Systems, Inc. All rights reserved. * Copyright (c) 2011, 2014 by Delphix. All rights reserved. * Copyright (c) 2013, Joyent, Inc. All rights reserved. */ @@ -1682,7 +1682,7 @@ zpool_import_props(libzfs_handle_t *hdl, nvlist_t *config, const char *newname, thename = origname; } - if (props) { + if (props != NULL) { uint64_t version; prop_flags_t flags = { .create = B_FALSE, .import = B_TRUE }; @@ -1690,12 +1690,13 @@ zpool_import_props(libzfs_handle_t *hdl, nvlist_t *config, const char *newname, &version) == 0); if ((props = zpool_valid_proplist(hdl, origname, - props, version, flags, errbuf)) == NULL) { + props, version, flags, errbuf)) == NULL) return (-1); - } else if (zcmd_write_src_nvlist(hdl, &zc, props) != 0) { + if (zcmd_write_src_nvlist(hdl, &zc, props) != 0) { nvlist_free(props); return (-1); } + nvlist_free(props); } (void) strlcpy(zc.zc_name, thename, sizeof (zc.zc_name)); @@ -1704,11 +1705,11 @@ zpool_import_props(libzfs_handle_t *hdl, nvlist_t *config, const char *newname, &zc.zc_guid) == 0); if (zcmd_write_conf_nvlist(hdl, &zc, config) != 0) { - nvlist_free(props); + zcmd_free_nvlists(&zc); return (-1); } if (zcmd_alloc_dst_nvlist(hdl, &zc, zc.zc_nvlist_conf_size * 2) != 0) { - nvlist_free(props); + zcmd_free_nvlists(&zc); return (-1); } @@ -1724,6 +1725,9 @@ zpool_import_props(libzfs_handle_t *hdl, nvlist_t *config, const char *newname, error = errno; (void) zcmd_read_dst_nvlist(hdl, &zc, &nv); + + zcmd_free_nvlists(&zc); + zpool_get_rewind_policy(config, &policy); if (error) { @@ -1829,9 +1833,6 @@ zpool_import_props(libzfs_handle_t *hdl, nvlist_t *config, const char *newname, return (0); } - zcmd_free_nvlists(&zc); - nvlist_free(props); - return (ret); } @@ -3281,8 +3282,10 @@ devid_to_path(char *devid_str) if (ret != 0) return (NULL); - if ((path = strdup(list[0].devname)) == NULL) - return (NULL); + /* + * In a case the strdup() fails, we will just return NULL below. + */ + path = strdup(list[0].devname); devid_free_nmlist(list);