Illumos 5518 - Memory leaks in libzfs import implementation

5518 Memory leaks in libzfs import implementation
Reviewed by: Dan Fields <dan.fields@nexenta.com>
Reviewed by: Matthew Ahrens <mahrens@delphix.com>
Reviewed by: Serghei Samsi <sscdvp@gmail.com>
Approved by: Dan McDonald <danmcd@omniti.com>

References:
  https://www.illumos.org/issues/5518
  https://github.com/illumos/illumos-gate/commit/078266a

Porting notes:
- One hunk of this change was already applied independently in
  commit 4def05f.

Ported-by: Brian Behlendorf <behlendorf1@llnl.gov>

diff --git a/lib/libzfs/libzfs_import.c b/lib/libzfs/libzfs_import.c
index 5da42cb5e217fce8a6ad9992c835a432439bb014..a834c3ea999ac4c0a431137ecc750d370274a18e 100644 (file)
--- a/lib/libzfs/libzfs_import.c
+++ b/lib/libzfs/libzfs_import.c
@@ -19,9 +19,9 @@
  * 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) 2012 by Delphix. All rights reserved.
- * Copyright 2014 Nexenta Systems, Inc. All rights reserved.
  */
 
 /*
@@ -203,8 +203,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);
        }
 
@@ -675,8 +677,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;
                        }
                }
@@ -1295,7 +1299,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;
@@ -1336,6 +1339,8 @@ zpool_find_import_impl(libzfs_handle_t *hdl, importargs_t *iarg)
                taskq_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) {
@@ -1366,6 +1371,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'"),
@@ -1417,7 +1424,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;
 
@@ -1438,14 +1445,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, i+1,
+                                           slice->rn_num_labels, 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, i+1,
-                                   slice->rn_num_labels, config))
-                                       goto error;
                        }
                        free(slice->rn_name);
                        free(slice);
@@ -1453,7 +1462,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;
        }
 
 #ifdef HAVE_LIBBLKID
@@ -1479,14 +1490,10 @@ error:
 
        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);
 }
 
@@ -1844,9 +1851,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;
 
@@ -1860,9 +1867,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/lib/libzfs/libzfs_pool.c b/lib/libzfs/libzfs_pool.c
index e603526f48d480e60f7d3f00a899b6bb38cb7ffb..9a276d061d35f5eaa12aa8e25462d37e40674ad0 100644 (file)
--- a/lib/libzfs/libzfs_pool.c
+++ b/lib/libzfs/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.
  */
 
@@ -1783,7 +1783,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 };
 
@@ -1791,12 +1791,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));
@@ -1805,11 +1806,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);
        }
 
@@ -1825,6 +1826,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) {
@@ -1936,9 +1940,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);
 }
 
@@ -3335,8 +3336,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);