svn commit: r337182 - vendor-sys/illumos/dist/common/zfs vendor-sys/illumos/dist/uts/common/fs/zfs vendor/illumos/dist/lib/libzfs/common vendor/illumos/dist/man/man1m
Alexander Motin
mav at FreeBSD.org
Thu Aug 2 21:12:54 UTC 2018
Author: mav
Date: Thu Aug 2 21:12:52 2018
New Revision: 337182
URL: https://svnweb.freebsd.org/changeset/base/337182
Log:
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:
vendor-sys/illumos/dist/common/zfs/zfs_namecheck.c
vendor-sys/illumos/dist/common/zfs/zfs_namecheck.h
vendor-sys/illumos/dist/uts/common/fs/zfs/dmu_objset.c
vendor-sys/illumos/dist/uts/common/fs/zfs/dsl_dir.c
Changes in other areas also in this revision:
Modified:
vendor/illumos/dist/lib/libzfs/common/libzfs_dataset.c
vendor/illumos/dist/man/man1m/zfs.1m
Modified: vendor-sys/illumos/dist/common/zfs/zfs_namecheck.c
==============================================================================
--- vendor-sys/illumos/dist/common/zfs/zfs_namecheck.c Thu Aug 2 21:07:04 2018 (r337181)
+++ vendor-sys/illumos/dist/common/zfs/zfs_namecheck.c Thu Aug 2 21:12:52 2018 (r337182)
@@ -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: vendor-sys/illumos/dist/common/zfs/zfs_namecheck.h
==============================================================================
--- vendor-sys/illumos/dist/common/zfs/zfs_namecheck.h Thu Aug 2 21:07:04 2018 (r337181)
+++ vendor-sys/illumos/dist/common/zfs/zfs_namecheck.h Thu Aug 2 21:12:52 2018 (r337182)
@@ -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: vendor-sys/illumos/dist/uts/common/fs/zfs/dmu_objset.c
==============================================================================
--- vendor-sys/illumos/dist/uts/common/fs/zfs/dmu_objset.c Thu Aug 2 21:07:04 2018 (r337181)
+++ vendor-sys/illumos/dist/uts/common/fs/zfs/dmu_objset.c Thu Aug 2 21:12:52 2018 (r337182)
@@ -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
@@ -909,6 +910,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: vendor-sys/illumos/dist/uts/common/fs/zfs/dsl_dir.c
==============================================================================
--- vendor-sys/illumos/dist/uts/common/fs/zfs/dsl_dir.c Thu Aug 2 21:07:04 2018 (r337181)
+++ vendor-sys/illumos/dist/uts/common/fs/zfs/dsl_dir.c Thu Aug 2 21:12:52 2018 (r337182)
@@ -1810,17 +1810,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);
}
@@ -1830,9 +1842,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);
@@ -1861,10 +1873,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-vendor
mailing list