From 5ac95da7d61660aa299c287a39277cb0372be959 Mon Sep 17 00:00:00 2001 From: Serapheim Dimitropoulos Date: Mon, 12 Sep 2016 08:15:20 -0700 Subject: 9330 stack overflow when creating a deeply nested dataset Reviewed by: John Kennedy Reviewed by: Matt Ahrens Approved by: Garrett D'Amore --- usr/src/cmd/mdb/common/modules/zfs/zfs.c | 1 + usr/src/common/zfs/zfs_namecheck.c | 73 +++++++++++--- usr/src/common/zfs/zfs_namecheck.h | 6 +- usr/src/lib/libzfs/common/libzfs_dataset.c | 21 ++++ usr/src/man/man1m/zfs.1m | 3 +- usr/src/pkg/manifests/system-test-zfstest.mf | 3 + usr/src/test/zfs-tests/runfiles/delphix.run | 2 +- usr/src/test/zfs-tests/runfiles/omnios.run | 2 +- usr/src/test/zfs-tests/runfiles/openindiana.run | 2 +- .../functional/cli_root/zfs_create/zfs_create.cfg | 8 +- .../cli_root/zfs_create/zfs_create_009_neg.ksh | 3 +- .../cli_root/zfs_rename/zfs_rename_014_neg.ksh | 110 +++++++++++++++++++++ usr/src/uts/common/fs/zfs/dmu_objset.c | 4 + usr/src/uts/common/fs/zfs/dsl_dir.c | 31 +++++- 14 files changed, 245 insertions(+), 24 deletions(-) create mode 100644 usr/src/test/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename_014_neg.ksh diff --git a/usr/src/cmd/mdb/common/modules/zfs/zfs.c b/usr/src/cmd/mdb/common/modules/zfs/zfs.c index fd8897b03a..dcb61ec63b 100644 --- a/usr/src/cmd/mdb/common/modules/zfs/zfs.c +++ b/usr/src/cmd/mdb/common/modules/zfs/zfs.c @@ -576,6 +576,7 @@ zfs_params(uintptr_t addr, uint_t flags, int argc, const mdb_arg_t *argv) "zvol_maxphys", "zvol_unmap_enabled", "zvol_unmap_sync_enabled", + "zfs_max_dataset_nesting", }; for (int i = 0; i < sizeof (params) / sizeof (params[0]); i++) { diff --git a/usr/src/common/zfs/zfs_namecheck.c b/usr/src/common/zfs/zfs_namecheck.c index 0997512526..bad8f20e69 100644 --- a/usr/src/common/zfs/zfs_namecheck.c +++ b/usr/src/common/zfs/zfs_namecheck.c @@ -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) { @@ -59,11 +65,36 @@ valid_char(char c) c == '-' || c == '_' || c == '.' || c == ':' || 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_err_t *why, char *what) * 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) @@ -120,29 +153,41 @@ permset_namecheck(const char *path, namecheck_err_t *why, char *what) return (zfs_component_namecheck(&path[1], why, what)); } +/* + * 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 *why, char *what) 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 *why, char *what) 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 *why, char *what) * 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 *why) * 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) diff --git a/usr/src/common/zfs/zfs_namecheck.h b/usr/src/common/zfs/zfs_namecheck.h index db70641dba..527db92b0c 100644 --- a/usr/src/common/zfs/zfs_namecheck.h +++ b/usr/src/common/zfs/zfs_namecheck.h @@ -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 *); diff --git a/usr/src/lib/libzfs/common/libzfs_dataset.c b/usr/src/lib/libzfs/common/libzfs_dataset.c index f91b9ecf6a..c48f633355 100644 --- a/usr/src/lib/libzfs/common/libzfs_dataset.c +++ b/usr/src/lib/libzfs/common/libzfs_dataset.c @@ -3409,8 +3409,22 @@ zfs_create_ancestors(libzfs_handle_t *hdl, const char *path) { 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); @@ -3446,6 +3460,12 @@ zfs_create(libzfs_handle_t *hdl, const char *path, zfs_type_t type, 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); @@ -4233,6 +4253,7 @@ zfs_rename(zfs_handle_t *zhp, const char *target, boolean_t recursive, errbuf)); } } + if (!zfs_validate_name(hdl, target, zhp->zfs_type, B_TRUE)) return (zfs_error(hdl, EZFS_INVALIDNAME, errbuf)); } else { diff --git a/usr/src/man/man1m/zfs.1m b/usr/src/man/man1m/zfs.1m index c24db19a2d..62ffd63404 100644 --- a/usr/src/man/man1m/zfs.1m +++ b/usr/src/man/man1m/zfs.1m @@ -289,7 +289,8 @@ pool/{filesystem,volume,snapshot} .Pp where the maximum length of a dataset name is .Dv MAXNAMELEN -.Pq 256 bytes . +.Pq 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 -tag -width "file system" diff --git a/usr/src/pkg/manifests/system-test-zfstest.mf b/usr/src/pkg/manifests/system-test-zfstest.mf index c10c421246..5916d9079a 100644 --- a/usr/src/pkg/manifests/system-test-zfstest.mf +++ b/usr/src/pkg/manifests/system-test-zfstest.mf @@ -1011,6 +1011,9 @@ file \ file \ path=opt/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename_013_pos \ mode=0555 +file \ + path=opt/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename_014_neg \ + mode=0555 file path=opt/zfs-tests/tests/functional/cli_root/zfs_reservation/cleanup \ mode=0555 file path=opt/zfs-tests/tests/functional/cli_root/zfs_reservation/setup \ diff --git a/usr/src/test/zfs-tests/runfiles/delphix.run b/usr/src/test/zfs-tests/runfiles/delphix.run index b5974f8476..3210452f70 100644 --- a/usr/src/test/zfs-tests/runfiles/delphix.run +++ b/usr/src/test/zfs-tests/runfiles/delphix.run @@ -170,7 +170,7 @@ tests = ['zfs_rename_001_pos', 'zfs_rename_002_pos', 'zfs_rename_003_pos', 'zfs_rename_004_neg', 'zfs_rename_005_neg', 'zfs_rename_006_pos', 'zfs_rename_007_pos', 'zfs_rename_008_pos', 'zfs_rename_009_neg', 'zfs_rename_010_neg', 'zfs_rename_011_pos', 'zfs_rename_012_neg', - 'zfs_rename_013_pos'] + 'zfs_rename_013_pos', 'zfs_rename_014_neg'] [/opt/zfs-tests/tests/functional/cli_root/zfs_reservation] tests = ['zfs_reservation_001_pos', 'zfs_reservation_002_pos'] diff --git a/usr/src/test/zfs-tests/runfiles/omnios.run b/usr/src/test/zfs-tests/runfiles/omnios.run index 57a828c86f..eb14376f64 100644 --- a/usr/src/test/zfs-tests/runfiles/omnios.run +++ b/usr/src/test/zfs-tests/runfiles/omnios.run @@ -160,7 +160,7 @@ tests = ['zfs_rename_001_pos', 'zfs_rename_002_pos', 'zfs_rename_003_pos', 'zfs_rename_004_neg', 'zfs_rename_005_neg', 'zfs_rename_006_pos', 'zfs_rename_007_pos', 'zfs_rename_008_pos', 'zfs_rename_009_neg', 'zfs_rename_010_neg', 'zfs_rename_011_pos', 'zfs_rename_012_neg', - 'zfs_rename_013_pos'] + 'zfs_rename_013_pos', 'zfs_rename_014_neg'] [/opt/zfs-tests/tests/functional/cli_root/zfs_remap] tests = ['zfs_remap_cliargs', 'zfs_remap_obsolete_counts'] diff --git a/usr/src/test/zfs-tests/runfiles/openindiana.run b/usr/src/test/zfs-tests/runfiles/openindiana.run index 4cefe8f228..71096bdbdc 100644 --- a/usr/src/test/zfs-tests/runfiles/openindiana.run +++ b/usr/src/test/zfs-tests/runfiles/openindiana.run @@ -160,7 +160,7 @@ tests = ['zfs_rename_001_pos', 'zfs_rename_002_pos', 'zfs_rename_003_pos', 'zfs_rename_004_neg', 'zfs_rename_005_neg', 'zfs_rename_006_pos', 'zfs_rename_007_pos', 'zfs_rename_008_pos', 'zfs_rename_009_neg', 'zfs_rename_010_neg', 'zfs_rename_011_pos', 'zfs_rename_012_neg', - 'zfs_rename_013_pos'] + 'zfs_rename_013_pos', 'zfs_rename_014_neg'] [/opt/zfs-tests/tests/functional/cli_root/zfs_remap] tests = ['zfs_remap_cliargs', 'zfs_remap_obsolete_counts'] diff --git a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_create/zfs_create.cfg b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_create/zfs_create.cfg index c97e44d879..b96908ce12 100644 --- a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_create/zfs_create.cfg +++ b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_create/zfs_create.cfg @@ -25,7 +25,7 @@ # # -# Copyright (c) 2012 by Delphix. All rights reserved. +# Copyright (c) 2012, 2016 by Delphix. All rights reserved. # export BYND_MAX_NAME="byondmaxnamelength\ @@ -38,6 +38,12 @@ export BYND_MAX_NAME="byondmaxnamelength\ 012345678901234567890123456789\ 012345678901234567890123456789" +export BYND_NEST_LIMIT="a/a/a/\ +a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/\ +a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/\ +a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/\ +a/a" + # There're 3 different prompt messages while create # a volume that great than 1TB on 32-bit # - volume size exceeds limit for this system. (happy gate) diff --git a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_create/zfs_create_009_neg.ksh b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_create/zfs_create_009_neg.ksh index 28c6f0cc2d..b8190626c7 100644 --- a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_create/zfs_create_009_neg.ksh +++ b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_create/zfs_create_009_neg.ksh @@ -43,6 +43,7 @@ # *Beyond maximal name length. # *Same property set multiple times via '-o property=value' # *Volume's property set on filesystem +# *Exceeding maximum name nesting # # STRATEGY: # 1. Create an array of arguments @@ -89,7 +90,7 @@ set -A args "$TESTPOOL/" "$TESTPOOL//blah" "$TESTPOOL/@blah" \ "$TESTPOOL/blah*blah" "$TESTPOOL/blah blah" \ "-s $TESTPOOL/$TESTFS1" "-b 1092 $TESTPOOL/$TESTFS1" \ "-b 64k $TESTPOOL/$TESTFS1" "-s -b 32k $TESTPOOL/$TESTFS1" \ - "$TESTPOOL/$BYND_MAX_NAME" + "$TESTPOOL/$BYND_MAX_NAME" "$TESTPOOL/$BYND_NEST_LIMIT" log_assert "Verify 'zfs create ' fails with bad argument." diff --git a/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename_014_neg.ksh b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename_014_neg.ksh new file mode 100644 index 0000000000..6aaa107f61 --- /dev/null +++ b/usr/src/test/zfs-tests/tests/functional/cli_root/zfs_rename/zfs_rename_014_neg.ksh @@ -0,0 +1,110 @@ +#!/bin/ksh -p +# +# CDDL HEADER START +# +# The contents of this file are subject to the terms of the +# Common Development and Distribution License (the "License"). +# You may not use this file except in compliance with the License. +# +# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE +# or http://www.opensolaris.org/os/licensing. +# See the License for the specific language governing permissions +# and limitations under the License. +# +# When distributing Covered Code, include this CDDL HEADER in each +# file and include the License file at usr/src/OPENSOLARIS.LICENSE. +# If applicable, add the following below this CDDL HEADER, with the +# fields enclosed by brackets "[]" replaced with your own identifying +# information: Portions Copyright [yyyy] [name of copyright owner] +# +# CDDL HEADER END +# + +# +# Copyright (c) 2016 by Delphix. All rights reserved. +# + +. $STF_SUITE/include/libtest.shlib + +# +# DESCRIPTION: +# zfs rename should work on existing datasets that exceed +# zfs_max_dataset_nesting (our nesting limit) except in the +# scenario that we try to rename it to something deeper +# than it already is. +# +# STRATEGY: +# 1. Create a set of ZFS datasets within our nesting limit. +# 2. Try renaming one of them on top of the other so its +# children pass the limit - it should fail. +# 3. Increase the nesting limit. +# 4. Check that renaming a dataset on top of the other +# cannot exceed the new nesting limit but can exceed +# the old one. +# 5. Bring back the old nesting limit so you can simulate +# the scenario of existing datasets that exceed our +# nesting limit. +# 6. Make sure that 'zfs rename' can work only if we are +# trying to keep existing datasets that exceed the limit +# at the same nesting level or less. Making it even +# deeper should not work. +# + +verify_runnable "both" + +dsA01="a" +dsA02="a/a" +dsA49=$(printf 'a/%.0s' {1..48})"a" + +dsB01="b" +dsB49=$(printf 'b/%.0s' {1..48})"b" + +dsC01="c" +dsC16=$(printf 'c/%.0s' {1..15})"c" + +dsB16A=$(printf 'b/%.0s' {1..16})"a" +dsB15A=$(printf 'b/%.0s' {1..15})"a" + +dsB15A47A=$(printf 'b/%.0s' {1..15})$(printf 'a/%.0s' {1..47})"a" +dsB15A47C=$(printf 'b/%.0s' {1..15})$(printf 'a/%.0s' {1..47})"c" + +dsB15A40B=$(printf 'b/%.0s' {1..15})$(printf 'a/%.0s' {1..40})"b" +dsB15A47B=$(printf 'b/%.0s' {1..15})$(printf 'a/%.0s' {1..47})"b" + +function nesting_cleanup +{ + log_must zfs destroy -fR $TESTPOOL/$dsA01 + log_must zfs destroy -fR $TESTPOOL/$dsB01 + log_must zfs destroy -fR $TESTPOOL/$dsC01 + + # If the test fails after increasing the limit and + # before resetting it, it will be left at the modified + # value for the remaining tests. That's the reason + # we reset it again here just in case. + mdb_set_uint32 zfs_max_dataset_nesting 50 +} + +log_onexit nesting_cleanup + +log_must zfs create -p $TESTPOOL/$dsA49 +log_must zfs create -p $TESTPOOL/$dsB49 +log_must zfs create -p $TESTPOOL/$dsC16 + +log_mustnot zfs rename $TESTPOOL/$dsA02 $TESTPOOL/$dsB15A + +# extend limit +mdb_set_uint32 zfs_max_dataset_nesting 64 + +log_mustnot zfs rename $TESTPOOL/$dsA02 $TESTPOOL/$dsB16A +log_must zfs rename $TESTPOOL/$dsA02 $TESTPOOL/$dsB15A + +# bring back old limit +mdb_set_uint32 zfs_max_dataset_nesting 50 + +log_mustnot zfs rename $TESTPOOL/$dsC01 $TESTPOOL/$dsB15A47C +log_must zfs rename $TESTPOOL/$dsB15A47A $TESTPOOL/$dsB15A47B +log_must zfs rename $TESTPOOL/$dsB15A47B $TESTPOOL/$dsB15A40B + +log_pass "Verify 'zfs rename' limits datasets so they don't pass " \ + "the nesting limit. For existing ones that do, it should " \ + "not allow them to grow anymore." diff --git a/usr/src/uts/common/fs/zfs/dmu_objset.c b/usr/src/uts/common/fs/zfs/dmu_objset.c index f345ed9383..c5267ac18d 100644 --- a/usr/src/uts/common/fs/zfs/dmu_objset.c +++ b/usr/src/uts/common/fs/zfs/dmu_objset.c @@ -54,6 +54,7 @@ #include #include #include +#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) 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); if (error != 0) return (error); diff --git a/usr/src/uts/common/fs/zfs/dsl_dir.c b/usr/src/uts/common/fs/zfs/dsl_dir.c index 59ecfa6438..e945e620f1 100644 --- a/usr/src/uts/common/fs/zfs/dsl_dir.c +++ b/usr/src/uts/common/fs/zfs/dsl_dir.c @@ -1810,16 +1810,28 @@ 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); -- cgit v1.2.3