Permalink
Browse files

ZVOLs should not be allowed to have children

zfs receive and zfs create can bypass this hierarchy rule. Update both
userland and kernel module to handle this and use pyzfs unit tests to
exercise the ioctls directly.

Note: this commit slightly changes zfs_ioc_create() ABI. This allow to
differentiate a generic error (EINVAL) from the specific case where we
tried to create a dataset below a ZVOL (ENOTDIR).

Signed-off-by: loli10K <ezomori.nozomu@gmail.com>
Requires-builders: style test
  • Loading branch information...
loli10K committed Dec 4, 2018
1 parent b53cb02 commit 4dbf032abbf17c294d2529ffc7af476ac5ace4e0
@@ -45,13 +45,14 @@ def lzc_create_translate_error(ret, name, ds_type, props):
if ret == 0:
return
if ret == errno.EINVAL:
# XXX: should raise lzc_exc.WrongParent if parent is ZVOL
_validate_fs_name(name)
raise lzc_exc.PropertyInvalid(name)
if ret == errno.EEXIST:
raise lzc_exc.FilesystemExists(name)
if ret == errno.ENOENT:
raise lzc_exc.ParentNotFound(name)
if ret == errno.ENOTDIR:
raise lzc_exc.WrongParent(_fs_name(name))
raise _generic_exception(ret, name, "Failed to create filesystem")
@@ -441,6 +442,8 @@ def _map(ret, name):
raise lzc_exc.SuspendedPool(_pool_name(snapname))
if ret == errno.EBADE: # ECKSUM
raise lzc_exc.BadStream()
if ret == errno.ENOTDIR:
raise lzc_exc.WrongParent(_fs_name(snapname))
raise lzc_exc.StreamIOError(ret)
@@ -752,6 +752,8 @@ def lzc_receive(snapname, fd, force=False, raw=False, origin=None, props=None):
supported on this side.
:raises NameInvalid: if the name of either snapshot is invalid.
:raises NameTooLong: if the name of either snapshot is too long.
:raises WrongParent: if the parent dataset of the received destination is
not a filesystem (e.g. ZVOL)
.. note::
The ``origin`` is ignored if the actual stream is an incremental stream
@@ -139,7 +139,7 @@ def __init__(self, name):
class WrongParent(ZFSError):
errno = errno.EINVAL
errno = errno.ENOTDIR
message = "Parent dataset is not a filesystem"
def __init__(self, name):

Large diffs are not rendered by default.

Oops, something went wrong.
@@ -3809,7 +3809,7 @@ zfs_create(libzfs_handle_t *hdl, const char *path, zfs_type_t type,
"no such parent '%s'"), parent);
return (zfs_error(hdl, EZFS_NOENT, errbuf));
case EINVAL:
case ENOTDIR:
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
"parent '%s' is not a filesystem"), parent);
return (zfs_error(hdl, EZFS_BADTYPE, errbuf));
@@ -3934,6 +3934,15 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap,
err = zfs_error(hdl, EZFS_EXISTS, errbuf);
goto out;
}
if (ioctl(hdl->libzfs_fd, ZFS_IOC_DATASET_LIST_NEXT,
&zc) == 0 && drrb->drr_type == DMU_OST_ZVOL) {
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
"destination has children (eg. %s)\n"
"cannot overwrite with a ZVOL"),
zc.zc_name);
err = zfs_error(hdl, EZFS_BADTYPE, errbuf);
goto out;
}
}
if ((zhp = zfs_open(hdl, name,
@@ -4028,6 +4037,8 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap,
zfs_close(zhp);
} else {
zfs_handle_t *zhp;
zfs_handle_t *pzhp;
char parent_name[ZFS_MAX_DATASET_NAME_LEN];
/*
* Destination filesystem does not exist. Therefore we better
@@ -4057,6 +4068,22 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap,
goto out;
}
/* validate parent */
zhp = zfs_open(hdl, name, ZFS_TYPE_DATASET);
if (zhp == NULL) {
err = zfs_error(hdl, EZFS_BADRESTORE, errbuf);
goto out;
}
(void) zfs_parent_name(zhp, parent_name, sizeof (parent_name));
pzhp = make_dataset_handle(zhp->zfs_hdl, parent_name);
if (pzhp == NULL || zfs_get_type(pzhp) != ZFS_TYPE_FILESYSTEM) {
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
"parent is not a filesystem"));
err = zfs_error(hdl, EZFS_BADTYPE, errbuf);
zfs_close(zhp);
goto out;
}
/*
* It is invalid to receive a properties stream that was
* unencrypted on the send side as a child of an encrypted
@@ -4074,23 +4101,18 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap,
zfs_prop_to_name(ZFS_PROP_ENCRYPTION))) {
uint64_t crypt;
zhp = zfs_open(hdl, name, ZFS_TYPE_DATASET);
if (zhp == NULL) {
err = zfs_error(hdl, EZFS_BADRESTORE, errbuf);
goto out;
}
crypt = zfs_prop_get_int(zhp, ZFS_PROP_ENCRYPTION);
zfs_close(zhp);
if (crypt != ZIO_CRYPT_OFF) {
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
"parent '%s' must not be encrypted to "
"receive unenecrypted property"), name);
err = zfs_error(hdl, EZFS_BADPROP, errbuf);
zfs_close(zhp);
goto out;
}
}
zfs_close(zhp);
newfs = B_TRUE;
*cp = '/';
@@ -445,6 +445,11 @@ zfs_standard_error_fmt(libzfs_handle_t *hdl, int error, const char *fmt, ...)
"pool I/O is currently suspended"));
zfs_verror(hdl, EZFS_POOLUNAVAIL, fmt, ap);
break;
case ENOTDIR:
zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
"parent is not a filesystem"));
zfs_verror(hdl, EZFS_BADTYPE, fmt, ap);
break;
case EREMOTEIO:
zfs_verror(hdl, EZFS_ACTIVE_POOL, fmt, ap);
break;
@@ -79,6 +79,7 @@ recv_begin_check_existing_impl(dmu_recv_begin_arg_t *drba, dsl_dataset_t *ds,
uint64_t fromguid, uint64_t featureflags)
{
uint64_t val;
uint64_t children;
int error;
dsl_pool_t *dp = ds->ds_dir->dd_pool;
boolean_t encrypted = ds->ds_dir->dd_crypto_obj != 0;
@@ -99,6 +100,15 @@ recv_begin_check_existing_impl(dmu_recv_begin_arg_t *drba, dsl_dataset_t *ds,
if (error != ENOENT)
return (error == 0 ? EEXIST : error);
/* must not have children if receiving a ZVOL */
error = zap_count(dp->dp_meta_objset,
dsl_dir_phys(ds->ds_dir)->dd_child_dir_zapobj, &children);
if (error != 0)
return (error);
if (drba->drba_cookie->drc_drrb->drr_type != DMU_OST_ZFS &&
children > 0)
return (SET_ERROR(ENOTDIR));
/*
* Check snapshot limit before receiving. We'll recheck again at the
* end, but might as well abort before receiving if we're already over
@@ -283,6 +293,7 @@ dmu_recv_begin_check(void *arg, dmu_tx_t *tx)
} else if (error == ENOENT) {
/* target fs does not exist; must be a full backup or clone */
char buf[ZFS_MAX_DATASET_NAME_LEN];
objset_t *os;
/*
* If it's a non-clone incremental, we are missing the
@@ -352,6 +363,17 @@ dmu_recv_begin_check(void *arg, dmu_tx_t *tx)
return (error);
}
/* can't recv below anything but filesystems (eg. no ZVOLs) */
error = dmu_objset_from_ds(ds, &os);
if (error != 0) {
dsl_dataset_rele_flags(ds, dsflags, FTAG);
return (error);
}
if (os->os_phys->os_type != DMU_OST_ZFS) {
dsl_dataset_rele_flags(ds, dsflags, FTAG);
return (SET_ERROR(ENOTDIR));
}
if (drba->drba_origin != NULL) {
dsl_dataset_t *origin;
@@ -381,6 +403,7 @@ dmu_recv_begin_check(void *arg, dmu_tx_t *tx)
dsl_dataset_rele_flags(origin,
dsflags, FTAG);
}
dsl_dataset_rele_flags(ds, dsflags, FTAG);
error = 0;
}
@@ -3080,8 +3080,9 @@ zfs_fill_zplprops_impl(objset_t *os, uint64_t zplver,
ASSERT(zplprops != NULL);
/* parent dataset must be a filesystem */
if (os != NULL && os->os_phys->os_type != DMU_OST_ZFS)
return (SET_ERROR(EINVAL));
return (SET_ERROR(ENOTDIR));
/*
* Pull out creator prop choices, if any.
@@ -3160,15 +3161,11 @@ zfs_fill_zplprops(const char *dataset, nvlist_t *createprops,
uint64_t zplver = ZPL_VERSION;
objset_t *os = NULL;
char parentname[ZFS_MAX_DATASET_NAME_LEN];
char *cp;
spa_t *spa;
uint64_t spa_vers;
int error;
(void) strlcpy(parentname, dataset, sizeof (parentname));
cp = strrchr(parentname, '/');
ASSERT(cp != NULL);
cp[0] = '\0';
zfs_get_parent(dataset, parentname, sizeof (parentname));
if ((error = spa_open(dataset, &spa, FTAG)) != 0)
return (error);
@@ -3238,6 +3235,8 @@ zfs_ioc_create(const char *fsname, nvlist_t *innvl, nvlist_t *outnvl)
dmu_objset_type_t type;
boolean_t is_insensitive = B_FALSE;
dsl_crypto_params_t *dcp = NULL;
objset_t *parentos;
char parentname[ZFS_MAX_DATASET_NAME_LEN];
type = (dmu_objset_type_t)fnvlist_lookup_int32(innvl, "type");
(void) nvlist_lookup_nvlist(innvl, "props", &nvprops);
@@ -3265,6 +3264,17 @@ zfs_ioc_create(const char *fsname, nvlist_t *innvl, nvlist_t *outnvl)
if (cbfunc == NULL)
return (SET_ERROR(EINVAL));
zfs_get_parent(fsname, parentname, sizeof (parentname));
error = dmu_objset_hold(parentname, FTAG, &parentos);
if (error != 0) {
return (error);
}
if (parentos->os_phys->os_type != DMU_OST_ZFS) {
dmu_objset_rele(parentos, FTAG);
return (SET_ERROR(ENOTDIR));
}
dmu_objset_rele(parentos, FTAG);
if (type == DMU_OST_ZVOL) {
uint64_t volsize, volblocksize;
@@ -206,7 +206,8 @@ tests = ['zfs_receive_001_pos', 'zfs_receive_002_pos', 'zfs_receive_003_pos',
'zfs_receive_013_pos', 'zfs_receive_014_pos', 'zfs_receive_015_pos',
'receive-o-x_props_override', 'zfs_receive_from_encrypted',
'zfs_receive_to_encrypted', 'zfs_receive_raw',
'zfs_receive_raw_incremental', 'zfs_receive_-e']
'zfs_receive_raw_incremental', 'zfs_receive_-e',
'zfs_receive_zvol_hierarchy']
tags = ['functional', 'cli_root', 'zfs_receive']
[tests/functional/cli_root/zfs_remap]
@@ -22,4 +22,5 @@ dist_pkgdata_SCRIPTS = \
zfs_receive_to_encrypted.ksh \
zfs_receive_raw.ksh \
zfs_receive_raw_incremental.ksh \
zfs_receive_-e.ksh
zfs_receive_-e.ksh \
zfs_receive_zvol_hierarchy.ksh
@@ -0,0 +1,82 @@
#!/bin/ksh -p
#
# CDDL HEADER START
#
# This file and its contents are supplied under the terms of the
# Common Development and Distribution License ("CDDL"), version 1.0.
# You may only use this file in accordance with the terms of version
# 1.0 of the CDDL.
#
# A full copy of the text of the CDDL should have accompanied this
# source. A copy of the CDDL is also available via the Internet at
# http://www.illumos.org/license/CDDL.
#
# CDDL HEADER END
#
#
# Copyright 2018, loli10K <ezomori.nozomu@gmail.com>. All rights reserved.
#
. $STF_SUITE/include/libtest.shlib
#
# DESCRIPTION:
# ZVOLs cannot have children datasets: 'zfs receive' should respect this
# hierarchy rule.
#
# STRATEGY:
# 1. Create a filesystem and a ZVOL to send
# 2. Verify 'zfs recv' will not (force-)receive a ZVOL over the root dataset
# 3. Verify 'zfs recv' cannot receive a ZVOL overwriting datasets with children
# 4. Verify 'zfs recv' cannot receive datasets below a ZVOL
#
verify_runnable "both"
function cleanup
{
destroy_pool "$poolname"
log_must rm -f "$vdevfile"
log_must rm -f "$streamfile"
}
log_assert "ZVOLs cannot have children datasets: 'zfs receive' should respect"\
"this hierarchy rule"
log_onexit cleanup
poolname="$TESTPOOL-zfsrecv"
sendvol="$poolname/sendvol"
sendfs="$poolname/sendfs"
vdevfile="$TEST_BASE_DIR/vdevfile.$$"
streamfile="$TEST_BASE_DIR/streamfile.$$"
# 1. Create a filesystem and a ZVOL to send
# NOTE: set "mountpoint=none" just to speed up the test process
log_must truncate -s $MINVDEVSIZE "$vdevfile"
log_must zpool create -O mountpoint=none "$poolname" "$vdevfile"
log_must zfs create -V 1M -s "$sendvol"
log_must zfs create "$sendfs"
log_must zfs snapshot "$sendvol@snap"
log_must zfs snapshot "$sendfs@snap"
# 2. Verify 'zfs recv' will not (force-)receive a ZVOL over the root dataset
log_must eval "zfs send $sendvol@snap > $streamfile"
log_mustnot eval "zfs receive -F $poolname < $streamfile"
# 3. Verify 'zfs recv' cannot receive a ZVOL overwriting datasets with children
log_must eval "zfs send $sendvol@snap > $streamfile"
log_must zfs create "$poolname/fs"
log_must zfs create "$poolname/fs/subfs"
log_mustnot eval "zfs receive -F $poolname/fs < $streamfile"
log_must zfs destroy "$poolname/fs/subfs"
log_must eval "zfs receive -F $poolname/fs < $streamfile"
# 4. Verify 'zfs recv' cannot receive datasets below a ZVOL
log_must eval "zfs send $sendfs@snap > $streamfile"
log_mustnot eval "zfs receive $sendvol/subfs < $streamfile"
log_must eval "zfs send $sendvol@snap > $streamfile"
log_mustnot eval "zfs receive $sendvol/subvol < $streamfile"
log_pass "ZVOLs cannot have children datasets and 'zfs receive' will enforce"\
"this rule"

0 comments on commit 4dbf032

Please sign in to comment.