svn commit: r337183 - in head: cddl/contrib/opensolaris/cmd/zfs cddl/contrib/opensolaris/lib/libzfs/common sys/cddl/contrib/opensolaris/common/zfs sys/cddl/contrib/opensolaris/uts/common/fs/zfs
Alexander Motin
mav at FreeBSD.org
Thu Aug 2 21:19:38 UTC 2018
Author: mav
Date: Thu Aug 2 21:19:35 2018
New Revision: 337183
URL: https://svnweb.freebsd.org/changeset/base/337183
Log:
MFV r337182: 9330 stack overflow when creating a deeply nested dataset
Datasets that are deeply nested (~100 levels) are impractical. We just put
a limit of 50 levels to newly created datasets. Existing datasets should
work without a problem.
illumos/illumos-gate at 5ac95da7d61660aa299c287a39277cb0372be959
Reviewed by: John Kennedy <john.kennedy at delphix.com>
Reviewed by: Matt Ahrens <matt at delphix.com>
Approved by: Garrett D'Amore <garrett at damore.org>
Author: Serapheim Dimitropoulos <serapheim.dimitro at delphix.com>
Modified:
head/cddl/contrib/opensolaris/cmd/zfs/zfs.8
head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c
head/sys/cddl/contrib/opensolaris/common/zfs/zfs_namecheck.c
head/sys/cddl/contrib/opensolaris/common/zfs/zfs_namecheck.h
head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_objset.c
head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dir.c
Directory Properties:
head/cddl/contrib/opensolaris/ (props changed)
head/sys/cddl/contrib/opensolaris/ (props changed)
Modified: head/cddl/contrib/opensolaris/cmd/zfs/zfs.8
==============================================================================
--- head/cddl/contrib/opensolaris/cmd/zfs/zfs.8 Thu Aug 2 21:12:52 2018 (r337182)
+++ head/cddl/contrib/opensolaris/cmd/zfs/zfs.8 Thu Aug 2 21:19:35 2018 (r337183)
@@ -320,7 +320,8 @@ namespace. For example:
.Pp
where the maximum length of a dataset name is
.Dv MAXNAMELEN
-(256 bytes).
+(256 bytes)
+and the maximum amount of nesting allowed in a path is 50 levels deep.
.Pp
A dataset can be one of the following:
.Bl -hang -width 12n
Modified: head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c
==============================================================================
--- head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c Thu Aug 2 21:12:52 2018 (r337182)
+++ head/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c Thu Aug 2 21:19:35 2018 (r337183)
@@ -3449,8 +3449,22 @@ zfs_create_ancestors(libzfs_handle_t *hdl, const char
{
int prefix;
char *path_copy;
+ char errbuf[1024];
int rc = 0;
+ (void) snprintf(errbuf, sizeof (errbuf), dgettext(TEXT_DOMAIN,
+ "cannot create '%s'"), path);
+
+ /*
+ * Check that we are not passing the nesting limit
+ * before we start creating any ancestors.
+ */
+ if (dataset_nestcheck(path) != 0) {
+ zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+ "maximum name nesting depth exceeded"));
+ return (zfs_error(hdl, EZFS_INVALIDNAME, errbuf));
+ }
+
if (check_parents(hdl, path, NULL, B_TRUE, &prefix) != 0)
return (-1);
@@ -3486,6 +3500,12 @@ zfs_create(libzfs_handle_t *hdl, const char *path, zfs
if (!zfs_validate_name(hdl, path, type, B_TRUE))
return (zfs_error(hdl, EZFS_INVALIDNAME, errbuf));
+ if (dataset_nestcheck(path) != 0) {
+ zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
+ "maximum name nesting depth exceeded"));
+ return (zfs_error(hdl, EZFS_INVALIDNAME, errbuf));
+ }
+
/* validate parents exist */
if (check_parents(hdl, path, &zoned, B_FALSE, NULL) != 0)
return (-1);
@@ -4286,6 +4306,7 @@ zfs_rename(zfs_handle_t *zhp, const char *source, cons
errbuf));
}
}
+
if (!zfs_validate_name(hdl, target, zhp->zfs_type, B_TRUE))
return (zfs_error(hdl, EZFS_INVALIDNAME, errbuf));
} else {
Modified: head/sys/cddl/contrib/opensolaris/common/zfs/zfs_namecheck.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/common/zfs/zfs_namecheck.c Thu Aug 2 21:12:52 2018 (r337182)
+++ head/sys/cddl/contrib/opensolaris/common/zfs/zfs_namecheck.c Thu Aug 2 21:19:35 2018 (r337183)
@@ -23,7 +23,7 @@
* Use is subject to license terms.
*/
/*
- * Copyright (c) 2013 by Delphix. All rights reserved.
+ * Copyright (c) 2013, 2016 by Delphix. All rights reserved.
*/
/*
@@ -34,8 +34,6 @@
* name is invalid. In the kernel, we only care whether it's valid or not.
* Each routine therefore takes a 'namecheck_err_t' which describes exactly why
* the name failed to validate.
- *
- * Each function returns 0 on success, -1 on error.
*/
#if defined(_KERNEL)
@@ -50,6 +48,14 @@
#include "zfs_namecheck.h"
#include "zfs_deleg.h"
+/*
+ * Deeply nested datasets can overflow the stack, so we put a limit
+ * in the amount of nesting a path can have. zfs_max_dataset_nesting
+ * can be tuned temporarily to fix existing datasets that exceed our
+ * predefined limit.
+ */
+int zfs_max_dataset_nesting = 50;
+
static int
valid_char(char c)
{
@@ -60,10 +66,35 @@ valid_char(char c)
}
/*
+ * Looks at a path and returns its level of nesting (depth).
+ */
+int
+get_dataset_depth(const char *path)
+{
+ const char *loc = path;
+ int nesting = 0;
+
+ /*
+ * Keep track of nesting until you hit the end of the
+ * path or found the snapshot/bookmark seperator.
+ */
+ for (int i = 0; loc[i] != '\0' &&
+ loc[i] != '@' &&
+ loc[i] != '#'; i++) {
+ if (loc[i] == '/')
+ nesting++;
+ }
+
+ return (nesting);
+}
+
+/*
* Snapshot names must be made up of alphanumeric characters plus the following
* characters:
*
- * [-_.: ]
+ * [-_.: ]
+ *
+ * Returns 0 on success, -1 on error.
*/
int
zfs_component_namecheck(const char *path, namecheck_err_t *why, char *what)
@@ -99,6 +130,8 @@ zfs_component_namecheck(const char *path, namecheck_er
* Permissions set name must start with the letter '@' followed by the
* same character restrictions as snapshot names, except that the name
* cannot exceed 64 characters.
+ *
+ * Returns 0 on success, -1 on error.
*/
int
permset_namecheck(const char *path, namecheck_err_t *why, char *what)
@@ -121,28 +154,40 @@ permset_namecheck(const char *path, namecheck_err_t *w
}
/*
+ * Dataset paths should not be deeper than zfs_max_dataset_nesting
+ * in terms of nesting.
+ *
+ * Returns 0 on success, -1 on error.
+ */
+int
+dataset_nestcheck(const char *path)
+{
+ return ((get_dataset_depth(path) < zfs_max_dataset_nesting) ? 0 : -1);
+}
+
+/*
* Entity names must be of the following form:
*
- * [component/]*[component][(@|#)component]?
+ * [component/]*[component][(@|#)component]?
*
* Where each component is made up of alphanumeric characters plus the following
* characters:
*
- * [-_.:%]
+ * [-_.:%]
*
* We allow '%' here as we use that character internally to create unique
* names for temporary clones (for online recv).
+ *
+ * Returns 0 on success, -1 on error.
*/
int
entity_namecheck(const char *path, namecheck_err_t *why, char *what)
{
- const char *start, *end;
- int found_delim;
+ const char *end;
/*
* Make sure the name is not too long.
*/
-
if (strlen(path) >= ZFS_MAX_DATASET_NAME_LEN) {
if (why)
*why = NAME_ERR_TOOLONG;
@@ -162,8 +207,8 @@ entity_namecheck(const char *path, namecheck_err_t *wh
return (-1);
}
- start = path;
- found_delim = 0;
+ const char *start = path;
+ boolean_t found_delim = B_FALSE;
for (;;) {
/* Find the end of this component */
end = start;
@@ -198,7 +243,7 @@ entity_namecheck(const char *path, namecheck_err_t *wh
return (-1);
}
- found_delim = 1;
+ found_delim = B_TRUE;
}
/* Zero-length components are not allowed */
@@ -250,6 +295,8 @@ dataset_namecheck(const char *path, namecheck_err_t *w
* mountpoint names must be of the following form:
*
* /[component][/]*[component][/]
+ *
+ * Returns 0 on success, -1 on error.
*/
int
mountpoint_namecheck(const char *path, namecheck_err_t *why)
@@ -294,6 +341,8 @@ mountpoint_namecheck(const char *path, namecheck_err_t
* dataset names, with the additional restriction that the pool name must begin
* with a letter. The pool names 'raidz' and 'mirror' are also reserved names
* that cannot be used.
+ *
+ * Returns 0 on success, -1 on error.
*/
int
pool_namecheck(const char *pool, namecheck_err_t *why, char *what)
Modified: head/sys/cddl/contrib/opensolaris/common/zfs/zfs_namecheck.h
==============================================================================
--- head/sys/cddl/contrib/opensolaris/common/zfs/zfs_namecheck.h Thu Aug 2 21:12:52 2018 (r337182)
+++ head/sys/cddl/contrib/opensolaris/common/zfs/zfs_namecheck.h Thu Aug 2 21:19:35 2018 (r337183)
@@ -23,7 +23,7 @@
* Use is subject to license terms.
*/
/*
- * Copyright (c) 2013 by Delphix. All rights reserved.
+ * Copyright (c) 2013, 2016 by Delphix. All rights reserved.
*/
#ifndef _ZFS_NAMECHECK_H
@@ -48,9 +48,13 @@ typedef enum {
#define ZFS_PERMSET_MAXLEN 64
+extern int zfs_max_dataset_nesting;
+
+int get_dataset_depth(const char *);
int pool_namecheck(const char *, namecheck_err_t *, char *);
int entity_namecheck(const char *, namecheck_err_t *, char *);
int dataset_namecheck(const char *, namecheck_err_t *, char *);
+int dataset_nestcheck(const char *);
int mountpoint_namecheck(const char *, namecheck_err_t *);
int zfs_component_namecheck(const char *, namecheck_err_t *, char *);
int permset_namecheck(const char *, namecheck_err_t *, char *);
Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_objset.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_objset.c Thu Aug 2 21:12:52 2018 (r337182)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_objset.c Thu Aug 2 21:19:35 2018 (r337183)
@@ -54,6 +54,7 @@
#include <sys/dsl_destroy.h>
#include <sys/vdev.h>
#include <sys/zfeature.h>
+#include "zfs_namecheck.h"
/*
* Needed to close a window in dnode_move() that allows the objset to be freed
@@ -911,6 +912,9 @@ dmu_objset_create_check(void *arg, dmu_tx_t *tx)
return (SET_ERROR(EINVAL));
if (strlen(doca->doca_name) >= ZFS_MAX_DATASET_NAME_LEN)
+ return (SET_ERROR(ENAMETOOLONG));
+
+ if (dataset_nestcheck(doca->doca_name) != 0)
return (SET_ERROR(ENAMETOOLONG));
error = dsl_dir_hold(dp, doca->doca_name, FTAG, &pdd, &tail);
Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dir.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dir.c Thu Aug 2 21:12:52 2018 (r337182)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dir.c Thu Aug 2 21:19:35 2018 (r337183)
@@ -1819,17 +1819,29 @@ typedef struct dsl_dir_rename_arg {
cred_t *ddra_cred;
} dsl_dir_rename_arg_t;
+typedef struct dsl_valid_rename_arg {
+ int char_delta;
+ int nest_delta;
+} dsl_valid_rename_arg_t;
+
/* ARGSUSED */
static int
dsl_valid_rename(dsl_pool_t *dp, dsl_dataset_t *ds, void *arg)
{
- int *deltap = arg;
+ dsl_valid_rename_arg_t *dvra = arg;
char namebuf[ZFS_MAX_DATASET_NAME_LEN];
dsl_dataset_name(ds, namebuf);
- if (strlen(namebuf) + *deltap >= ZFS_MAX_DATASET_NAME_LEN)
+ ASSERT3U(strnlen(namebuf, ZFS_MAX_DATASET_NAME_LEN),
+ <, ZFS_MAX_DATASET_NAME_LEN);
+ int namelen = strlen(namebuf) + dvra->char_delta;
+ int depth = get_dataset_depth(namebuf) + dvra->nest_delta;
+
+ if (namelen >= ZFS_MAX_DATASET_NAME_LEN)
return (SET_ERROR(ENAMETOOLONG));
+ if (dvra->nest_delta > 0 && depth >= zfs_max_dataset_nesting)
+ return (SET_ERROR(ENAMETOOLONG));
return (0);
}
@@ -1839,9 +1851,9 @@ dsl_dir_rename_check(void *arg, dmu_tx_t *tx)
dsl_dir_rename_arg_t *ddra = arg;
dsl_pool_t *dp = dmu_tx_pool(tx);
dsl_dir_t *dd, *newparent;
+ dsl_valid_rename_arg_t dvra;
const char *mynewname;
int error;
- int delta = strlen(ddra->ddra_newname) - strlen(ddra->ddra_oldname);
/* target dir should exist */
error = dsl_dir_hold(dp, ddra->ddra_oldname, FTAG, &dd, NULL);
@@ -1870,10 +1882,19 @@ dsl_dir_rename_check(void *arg, dmu_tx_t *tx)
return (SET_ERROR(EEXIST));
}
+ ASSERT3U(strnlen(ddra->ddra_newname, ZFS_MAX_DATASET_NAME_LEN),
+ <, ZFS_MAX_DATASET_NAME_LEN);
+ ASSERT3U(strnlen(ddra->ddra_oldname, ZFS_MAX_DATASET_NAME_LEN),
+ <, ZFS_MAX_DATASET_NAME_LEN);
+ dvra.char_delta = strlen(ddra->ddra_newname)
+ - strlen(ddra->ddra_oldname);
+ dvra.nest_delta = get_dataset_depth(ddra->ddra_newname)
+ - get_dataset_depth(ddra->ddra_oldname);
+
/* if the name length is growing, validate child name lengths */
- if (delta > 0) {
+ if (dvra.char_delta > 0 || dvra.nest_delta > 0) {
error = dmu_objset_find_dp(dp, dd->dd_object, dsl_valid_rename,
- &delta, DS_FIND_CHILDREN | DS_FIND_SNAPSHOTS);
+ &dvra, DS_FIND_CHILDREN | DS_FIND_SNAPSHOTS);
if (error != 0) {
dsl_dir_rele(newparent, FTAG);
dsl_dir_rele(dd, FTAG);
More information about the svn-src-head
mailing list