Mercurial > illumos > git > illumos-gate
changeset 19434:d591e9494f1f
12260 Fix zpool history unbounded memory usage
Portions contributed by: Jerry Jelinek <jerry.jelinek@joyent.com>
Reviewed by: Tom Caputi <tcaputi@datto.com>
Reviewed by: Matt Ahrens <matt@delphix.com>
Reviewed by: Igor Kozhukhov <igor@dilos.org>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed by: Sanjay Nadkarni <Sanjay G Nadkarni <snadkarni@tintri.com>
Approved by: Dan McDonald <danmcd@joyent.com>
| author | Chunwei Chen <david.chen@nutanix.com> |
|---|---|
| date | Thu, 30 Jan 2020 12:57:07 -0700 |
| parents | 7c41859fb95d |
| children | 9c3e8d0fa5b3 |
| files | usr/src/cmd/zpool/zpool_main.c usr/src/lib/libzfs/common/libzfs.h usr/src/lib/libzfs/common/libzfs_pool.c |
| diffstat | 3 files changed, 42 insertions(+), 27 deletions(-) [+] |
line wrap: on
line diff
--- a/usr/src/cmd/zpool/zpool_main.c Sun Jan 26 11:32:45 2020 +0000 +++ b/usr/src/cmd/zpool/zpool_main.c Thu Jan 30 12:57:07 2020 -0700 @@ -6529,24 +6529,12 @@ boolean_t internal; } hist_cbdata_t; -/* - * Print out the command history for a specific pool. - */ -static int -get_history_one(zpool_handle_t *zhp, void *data) +static void +print_history_records(nvlist_t *nvhis, hist_cbdata_t *cb) { - nvlist_t *nvhis; nvlist_t **records; uint_t numrecords; - int ret, i; - hist_cbdata_t *cb = (hist_cbdata_t *)data; - - cb->first = B_FALSE; - - (void) printf(gettext("History for '%s':\n"), zpool_get_name(zhp)); - - if ((ret = zpool_get_history(zhp, &nvhis)) != 0) - return (ret); + int i; verify(nvlist_lookup_nvlist_array(nvhis, ZPOOL_HIST_RECORD, &records, &numrecords) == 0); @@ -6647,8 +6635,32 @@ (void) printf("]"); (void) printf("\n"); } +} + +/* + * Print out the command history for a specific pool. + */ +static int +get_history_one(zpool_handle_t *zhp, void *data) +{ + nvlist_t *nvhis; + int ret; + hist_cbdata_t *cb = (hist_cbdata_t *)data; + uint64_t off = 0; + boolean_t eof = B_FALSE; + + cb->first = B_FALSE; + + (void) printf(gettext("History for '%s':\n"), zpool_get_name(zhp)); + + while (!eof) { + if ((ret = zpool_get_history(zhp, &nvhis, &off, &eof)) != 0) + return (ret); + + print_history_records(nvhis, cb); + nvlist_free(nvhis); + } (void) printf("\n"); - nvlist_free(nvhis); return (ret); }
--- a/usr/src/lib/libzfs/common/libzfs.h Sun Jan 26 11:32:45 2020 +0000 +++ b/usr/src/lib/libzfs/common/libzfs.h Thu Jan 30 12:57:07 2020 -0700 @@ -459,7 +459,8 @@ extern char *zpool_vdev_name(libzfs_handle_t *, zpool_handle_t *, nvlist_t *, int name_flags); extern int zpool_upgrade(zpool_handle_t *, uint64_t); -extern int zpool_get_history(zpool_handle_t *, nvlist_t **); +extern int zpool_get_history(zpool_handle_t *, nvlist_t **, uint64_t *, + boolean_t *); extern int zpool_history_unpack(char *, uint64_t, uint64_t *, nvlist_t ***, uint_t *); extern void zpool_obj_to_path(zpool_handle_t *, uint64_t, uint64_t, char *,
--- a/usr/src/lib/libzfs/common/libzfs_pool.c Sun Jan 26 11:32:45 2020 +0000 +++ b/usr/src/lib/libzfs/common/libzfs_pool.c Thu Jan 30 12:57:07 2020 -0700 @@ -4342,33 +4342,37 @@ * Retrieve the command history of a pool. */ int -zpool_get_history(zpool_handle_t *zhp, nvlist_t **nvhisp) +zpool_get_history(zpool_handle_t *zhp, nvlist_t **nvhisp, uint64_t *off, + boolean_t *eof) { char *buf; int buflen = 128 * 1024; - uint64_t off = 0; nvlist_t **records = NULL; uint_t numrecords = 0; - int err, i; + int err = 0, i; + uint64_t start = *off; buf = malloc(buflen); if (buf == NULL) return (ENOMEM); - do { + /* process about 1MB a time */ + while (*off - start < 1024 * 1024) { uint64_t bytes_read = buflen; uint64_t leftover; - if ((err = get_history(zhp, buf, &off, &bytes_read)) != 0) + if ((err = get_history(zhp, buf, off, &bytes_read)) != 0) break; /* if nothing else was read in, we're at EOF, just return */ - if (!bytes_read) + if (!bytes_read) { + *eof = B_TRUE; break; + } if ((err = zpool_history_unpack(buf, bytes_read, &leftover, &records, &numrecords)) != 0) break; - off -= leftover; + *off -= leftover; if (leftover == bytes_read) { /* * no progress made, because buffer is not big enough @@ -4380,9 +4384,7 @@ if (buf == NULL) return (ENOMEM); } - - /* CONSTCOND */ - } while (1); + } free(buf);