Skip to content
Permalink
Browse files

9330 stack overflow when creating a deeply nested dataset

Reviewed by: John Kennedy <john.kennedy@delphix.com>
Reviewed by: Matt Ahrens <matt@delphix.com>
Approved by: Garrett D'Amore <garrett@damore.org>
  • Loading branch information...
sdimitro authored and prakashsurya committed Sep 12, 2016
1 parent 8dfe554 commit 5ac95da7d61660aa299c287a39277cb0372be959
@@ -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++) {
@@ -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)
@@ -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 *);
@@ -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 {
@@ -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"
@@ -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 \
@@ -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']
@@ -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']
@@ -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']
@@ -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)
@@ -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 <filesystem> 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 <filesystem>' fails with bad <filesystem> argument."

0 comments on commit 5ac95da

Please sign in to comment.
You can’t perform that action at this time.