From d9e1eec9a2a321295b78e74daea2b7cf5eff5f25 Mon Sep 17 00:00:00 2001 From: Paul Dagnelie Date: Wed, 15 Jun 2016 14:51:27 -0700 Subject: [PATCH] OpenZFS 6876 - Stack corruption after importing a pool with a too-long name Reviewed by: Prakash Surya Reviewed by: Dan Kimmel Reviewed by: George Wilson Reviewed by: Yuri Pankov Ported-by: Brian Behlendorf Calling dsl_dataset_name on a dataset with a 256 byte buffer is asking for trouble. We should check every dataset on import, using a 1024 byte buffer and checking each time to see if the dataset's new name is longer than 256 bytes. OpenZFS-issue: https://www.illumos.org/issues/6876 OpenZFS-commit: https://github.com/openzfs/openzfs/commit/ca8674e --- include/sys/dsl_dataset.h | 1 + include/sys/fs/zfs.h | 1 + lib/libzfs/libzfs_pool.c | 7 ++++++- module/zfs/dsl_dataset.c | 11 +++++++++++ module/zfs/spa.c | 18 ++++++++++++++++++ 5 files changed, 37 insertions(+), 1 deletion(-) diff --git a/include/sys/dsl_dataset.h b/include/sys/dsl_dataset.h index d6da5dcfdb..da6f21c2ee 100644 --- a/include/sys/dsl_dataset.h +++ b/include/sys/dsl_dataset.h @@ -221,6 +221,7 @@ int dsl_dataset_own_obj(struct dsl_pool *dp, uint64_t dsobj, void dsl_dataset_disown(dsl_dataset_t *ds, void *tag); void dsl_dataset_name(dsl_dataset_t *ds, char *name); boolean_t dsl_dataset_tryown(dsl_dataset_t *ds, void *tag); +int dsl_dataset_namelen(dsl_dataset_t *ds); uint64_t dsl_dataset_create_sync(dsl_dir_t *pds, const char *lastname, dsl_dataset_t *origin, uint64_t flags, cred_t *, dmu_tx_t *); uint64_t dsl_dataset_create_sync_dd(dsl_dir_t *dd, dsl_dataset_t *origin, diff --git a/include/sys/fs/zfs.h b/include/sys/fs/zfs.h index 4da144c724..57bf55f936 100644 --- a/include/sys/fs/zfs.h +++ b/include/sys/fs/zfs.h @@ -69,6 +69,7 @@ typedef enum dmu_objset_type { #define ZAP_MAXNAMELEN 256 #define ZAP_MAXVALUELEN (1024 * 8) #define ZAP_OLDMAXVALUELEN 1024 +#define ZFS_MAX_DATASET_NAME_LEN 256 /* * Dataset properties are identified by these constants and must be added to diff --git a/lib/libzfs/libzfs_pool.c b/lib/libzfs/libzfs_pool.c index a194b8b57e..da980cfd4f 100644 --- a/lib/libzfs/libzfs_pool.c +++ b/lib/libzfs/libzfs_pool.c @@ -1891,7 +1891,12 @@ zpool_import_props(libzfs_handle_t *hdl, nvlist_t *config, const char *newname, "one or more devices are already in use\n")); (void) zfs_error(hdl, EZFS_BADDEV, desc); break; - + case ENAMETOOLONG: + zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, + "new name of at least one dataset is longer than " + "the maximum allowable length")); + (void) zfs_error(hdl, EZFS_NAMETOOLONG, desc); + break; default: (void) zpool_standard_error(hdl, error, desc); zpool_explain_recover(hdl, diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c index a06388566e..7124f3d6a8 100644 --- a/module/zfs/dsl_dataset.c +++ b/module/zfs/dsl_dataset.c @@ -665,6 +665,17 @@ dsl_dataset_name(dsl_dataset_t *ds, char *name) } } +int +dsl_dataset_namelen(dsl_dataset_t *ds) +{ + int len; + VERIFY0(dsl_dataset_get_snapname(ds)); + mutex_enter(&ds->ds_lock); + len = dsl_dir_namelen(ds->ds_dir) + 1 + strlen(ds->ds_snapname); + mutex_exit(&ds->ds_lock); + return (len); +} + void dsl_dataset_rele(dsl_dataset_t *ds, void *tag) { diff --git a/module/zfs/spa.c b/module/zfs/spa.c index 4468cdf88a..a220ae393e 100644 --- a/module/zfs/spa.c +++ b/module/zfs/spa.c @@ -1974,6 +1974,16 @@ spa_load_verify_cb(spa_t *spa, zilog_t *zilog, const blkptr_t *bp, return (0); } +/* ARGSUSED */ +int +verify_dataset_name_len(dsl_pool_t *dp, dsl_dataset_t *ds, void *arg) +{ + if (dsl_dataset_namelen(ds) >= ZFS_MAX_DATASET_NAME_LEN) + return (SET_ERROR(ENAMETOOLONG)); + + return (0); +} + static int spa_load_verify(spa_t *spa) { @@ -1988,6 +1998,14 @@ spa_load_verify(spa_t *spa) if (policy.zrp_request & ZPOOL_NEVER_REWIND) return (0); + dsl_pool_config_enter(spa->spa_dsl_pool, FTAG); + error = dmu_objset_find_dp(spa->spa_dsl_pool, + spa->spa_dsl_pool->dp_root_dir_obj, verify_dataset_name_len, NULL, + DS_FIND_CHILDREN); + dsl_pool_config_exit(spa->spa_dsl_pool, FTAG); + if (error != 0) + return (error); + rio = zio_root(spa, NULL, &sle, ZIO_FLAG_CANFAIL | ZIO_FLAG_SPECULATIVE);