From 69534635ff48af494aaef3c8d6c9381577f7b423 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Wed, 25 Mar 2020 15:56:18 +0000 Subject: MFOpenZFS: ZVOLs should not be allowed to have children zfs create, receive and rename can bypass this hierarchy rule. Update both userland and kernel module to prevent this issue 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 (ZFS_ERR_WRONG_PARENT). Reviewed-by: Paul Dagnelie Reviewed-by: Matt Ahrens Reviewed-by: Brian Behlendorf Reviewed-by: Tom Caputi Signed-off-by: loli10K Approved by: mav (mentor) MFC after: 2 weeks Sponsored by: iXsystems, Inc. openzfs/zfs@d8d418ff0cc90776182534bce10b01e9487b63e4 --- .../contrib/opensolaris/lib/libzfs/common/libzfs.h | 1 + .../opensolaris/lib/libzfs/common/libzfs_dataset.c | 5 --- .../lib/libzfs/common/libzfs_sendrecv.c | 38 ++++++++++++++++++++++ .../opensolaris/lib/libzfs/common/libzfs_util.c | 5 +++ 4 files changed, 44 insertions(+), 5 deletions(-) (limited to 'cddl') diff --git a/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h b/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h index 0e53743b1a00..f5dbe7209db6 100644 --- a/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h +++ b/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h @@ -141,6 +141,7 @@ typedef enum zfs_error { EZFS_TOOMANY, /* argument list too long */ EZFS_INITIALIZING, /* currently initializing */ EZFS_NO_INITIALIZE, /* no active initialize */ + EZFS_WRONG_PARENT, /* invalid parent dataset (e.g ZVOL) */ EZFS_UNKNOWN } zfs_error_t; diff --git a/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c b/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c index e0728e3b6285..7075d060c78d 100644 --- a/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c +++ b/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c @@ -3654,11 +3654,6 @@ 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: - zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, - "parent '%s' is not a filesystem"), parent); - return (zfs_error(hdl, EZFS_BADTYPE, errbuf)); - case ENOTSUP: zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, "pool must be upgraded to set this " diff --git a/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c b/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c index a4539f6d6e4b..e6f654b25b5f 100644 --- a/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c +++ b/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c @@ -28,6 +28,7 @@ * Copyright 2015, OmniTI Computer Consulting, Inc. All rights reserved. * Copyright (c) 2014 Integros [integros.com] * Copyright 2016 Igor Kozhukhov + * Copyright (c) 2018, loli10K . All rights reserved. * Copyright (c) 2019 Datto Inc. */ @@ -3375,6 +3376,7 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap, * - we are resuming a failed receive. */ if (stream_wantsnewfs) { + boolean_t is_volume = drrb->drr_type == DMU_OST_ZVOL; if (!flags->force) { zcmd_free_nvlists(&zc); zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, @@ -3392,6 +3394,25 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap, zc.zc_name); return (zfs_error(hdl, EZFS_EXISTS, errbuf)); } + if (is_volume && strrchr(zc.zc_name, '/') == NULL) { + zcmd_free_nvlists(&zc); + zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, + "destination '%s' is the root dataset\n" + "cannot overwrite with a ZVOL"), + zc.zc_name); + return (zfs_error(hdl, EZFS_EXISTS, errbuf)); + } + if (is_volume && + ioctl(hdl->libzfs_fd, ZFS_IOC_DATASET_LIST_NEXT, + &zc) == 0) { + zcmd_free_nvlists(&zc); + zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, + "destination has children (eg. %s)\n" + "cannot overwrite with a ZVOL"), + zc.zc_name); + return (zfs_error(hdl, EZFS_WRONG_PARENT, + errbuf)); + } } if ((zhp = zfs_open(hdl, zc.zc_name, @@ -3441,6 +3462,8 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap, zfs_close(zhp); } else { + zfs_handle_t *zhp; + /* * Destination filesystem does not exist. Therefore we better * be creating a new filesystem (either from a full backup, or @@ -3468,6 +3491,21 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap, return (zfs_error(hdl, EZFS_BADRESTORE, errbuf)); } + /* validate parent */ + zhp = zfs_open(hdl, zc.zc_name, ZFS_TYPE_DATASET); + if (zhp == NULL) { + zcmd_free_nvlists(&zc); + return (zfs_error(hdl, EZFS_BADRESTORE, errbuf)); + } + if (zfs_get_type(zhp) != ZFS_TYPE_FILESYSTEM) { + zcmd_free_nvlists(&zc); + zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, + "parent '%s' is not a filesystem"), zc.zc_name); + zfs_close(zhp); + return (zfs_error(hdl, EZFS_WRONG_PARENT, errbuf)); + } + zfs_close(zhp); + newfs = B_TRUE; } diff --git a/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_util.c b/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_util.c index 8ba496abae33..e69467397fc8 100644 --- a/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_util.c +++ b/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_util.c @@ -265,6 +265,8 @@ libzfs_error_description(libzfs_handle_t *hdl) case EZFS_NO_INITIALIZE: return (dgettext(TEXT_DOMAIN, "there is no active " "initialization")); + case EZFS_WRONG_PARENT: + return (dgettext(TEXT_DOMAIN, "invalid parent dataset")); case EZFS_UNKNOWN: return (dgettext(TEXT_DOMAIN, "unknown error")); default: @@ -537,6 +539,9 @@ zpool_standard_error_fmt(libzfs_handle_t *hdl, int error, const char *fmt, ...) case ZFS_ERR_VDEV_TOO_BIG: zfs_verror(hdl, EZFS_VDEV_TOO_BIG, fmt, ap); break; + case ZFS_ERR_WRONG_PARENT: + zfs_verror(hdl, EZFS_WRONG_PARENT, fmt, ap); + break; default: zfs_error_aux(hdl, strerror(error)); zfs_verror(hdl, EZFS_UNKNOWN, fmt, ap); -- cgit v1.2.3