From owner-svn-src-head@freebsd.org Thu Jul 23 14:21:47 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 1FB2F37FC91; Thu, 23 Jul 2020 14:21:47 +0000 (UTC) (envelope-from markj@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4BCDzb05Ccz4cYL; Thu, 23 Jul 2020 14:21:47 +0000 (UTC) (envelope-from markj@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id C0B221A58F; Thu, 23 Jul 2020 14:21:46 +0000 (UTC) (envelope-from markj@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 06NELktX004072; Thu, 23 Jul 2020 14:21:46 GMT (envelope-from markj@FreeBSD.org) Received: (from markj@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 06NELkjZ004070; Thu, 23 Jul 2020 14:21:46 GMT (envelope-from markj@FreeBSD.org) Message-Id: <202007231421.06NELkjZ004070@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: markj set sender to markj@FreeBSD.org using -f From: Mark Johnston Date: Thu, 23 Jul 2020 14:21:46 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r363447 - in head/cddl/contrib/opensolaris: cmd/zpool lib/libzfs/common X-SVN-Group: head X-SVN-Commit-Author: markj X-SVN-Commit-Paths: in head/cddl/contrib/opensolaris: cmd/zpool lib/libzfs/common X-SVN-Commit-Revision: 363447 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 23 Jul 2020 14:21:47 -0000 Author: markj Date: Thu Jul 23 14:21:45 2020 New Revision: 363447 URL: https://svnweb.freebsd.org/changeset/base/363447 Log: MFOpenZFS: Fix zpool history unbounded memory usage In original implementation, zpool history will read the whole history before printing anything, causing memory usage goes unbounded. We fix this by breaking it into read-print iterations. Reviewed-by: Tom Caputi Reviewed-by: Matt Ahrens Reviewed-by: Igor Kozhukhov Reviewed-by: Brian Behlendorf Signed-off-by: Chunwei Chen Closes #9516 Note, this change changes the libzfs.so ABI by modifying the prototype of zpool_get_history(). Since libzfs is effectively private to the base system it is anticipated that this will not be a problem. PR: 247557 Obtained from: OpenZFS Reported and tested by: Sam Vaughan Discussed with: freqlabs MFC after: 2 weeks Differential Revision: https://reviews.freebsd.org/D25745 openzfs/zfs@7125a109dcc55628336ff3f58e59e503f4d7694d Modified: head/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c Modified: head/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c ============================================================================== --- head/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c Thu Jul 23 14:03:37 2020 (r363446) +++ head/cddl/contrib/opensolaris/cmd/zpool/zpool_main.c Thu Jul 23 14:21:45 2020 (r363447) @@ -6226,25 +6226,13 @@ typedef struct hist_cbdata { 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; + int i; - 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); - verify(nvlist_lookup_nvlist_array(nvhis, ZPOOL_HIST_RECORD, &records, &numrecords) == 0); for (i = 0; i < numrecords; i++) { @@ -6344,8 +6332,32 @@ get_history_one(zpool_handle_t *zhp, void *data) (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); } Modified: head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h ============================================================================== --- head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h Thu Jul 23 14:03:37 2020 (r363446) +++ head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h Thu Jul 23 14:21:45 2020 (r363447) @@ -441,7 +441,8 @@ typedef enum { 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 *, Modified: head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c ============================================================================== --- head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c Thu Jul 23 14:03:37 2020 (r363446) +++ head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_pool.c Thu Jul 23 14:21:45 2020 (r363447) @@ -4124,33 +4124,37 @@ zpool_history_unpack(char *buf, uint64_t bytes_read, u * 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; uint64_t buflen = HIS_BUF_LEN_DEF; - uint64_t off = 0; nvlist_t **records = NULL; uint_t numrecords = 0; int err, i; + uint64_t start = *off; buf = malloc(buflen); if (buf == NULL) return (ENOMEM); - do { + /* process about 1MB at 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 == 0) + if (bytes_read == 0) { + *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 @@ -4165,9 +4169,7 @@ zpool_get_history(zpool_handle_t *zhp, nvlist_t **nvhi break; } } - - /* CONSTCOND */ - } while (1); + } free(buf);