Skip to content

Commit

Permalink
5518 Memory leaks in libzfs import implementation
Browse files Browse the repository at this point in the history 
Reviewed by: Dan Fields <[email protected]>
Reviewed by: Matthew Ahrens <[email protected]>
Reviewed by: Serghei Samsi <[email protected]>
Approved by: Dan McDonald <[email protected]>
  • Loading branch information
@mtelka
mtelka authored and Dan McDonald committed Jan 14, 2015
1 parent 69a6f71 commit 078266a
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 33 deletions.
55 changes: 33 additions & 22 deletions usr/src/lib/libzfs/common/libzfs_import.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/

/*
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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'"),
Expand Down Expand Up @@ -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;

Expand All @@ -1244,21 +1254,26 @@ 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);
}
avl_destroy(&slice_cache);

(void) closedir(dirp);
dirp = NULL;

if (config_failed)
goto error;
}

ret = get_configs(hdl, &pools, iarg->can_be_active);
Expand All @@ -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);
}

Expand Down Expand Up @@ -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;

Expand All @@ -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;

Expand Down
25 changes: 14 additions & 11 deletions usr/src/lib/libzfs/common/libzfs_pool.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -1682,20 +1682,21 @@ 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 };

verify(nvlist_lookup_uint64(config, ZPOOL_CONFIG_VERSION,
&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));
Expand All @@ -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);
}

Expand All @@ -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) {
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit 078266a

Please sign in to comment.