d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

9330 stack overflow when creating a deeply nested dataset

Browse Source 
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@5ac95da7d6

Reviewed by: John Kennedy <john.kennedy@delphix.com>
Reviewed by: Matt Ahrens <matt@delphix.com>
Approved by: Garrett D'Amore <garrett@damore.org>
Author:     Serapheim Dimitropoulos <serapheim.dimitro@delphix.com>
This commit is contained in:
Alexander Motin 2018-08-02 21:12:52 +00:00
parent 8398b8e966
commit cb97edbcd0
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/vendor-sys/illumos/dist/; revision=337182
6 changed files with 119 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

73
common/zfs/zfs_namecheck.c
View File

@ -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)

6
common/zfs/zfs_namecheck.h
View File

@ -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 *);

21
lib/libzfs/common/libzfs_dataset.c
View File

@ -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 {

3
man/man1m/zfs.1m
View File

@ -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"

4
uts/common/fs/zfs/dmu_objset.c
View File

@ -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)
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);

31
uts/common/fs/zfs/dsl_dir.c
View File

@ -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);