From owner-svn-src-stable-11@freebsd.org Wed Apr 8 08:05:04 2020 Return-Path: Delivered-To: svn-src-stable-11@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 533452AD012; Wed, 8 Apr 2020 08:05:04 +0000 (UTC) (envelope-from freqlabs@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 48xxdr1rhdz3PqC; Wed, 8 Apr 2020 08:05:04 +0000 (UTC) (envelope-from freqlabs@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 363F81FFDA; Wed, 8 Apr 2020 08:05:04 +0000 (UTC) (envelope-from freqlabs@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 038854mD017144; Wed, 8 Apr 2020 08:05:04 GMT (envelope-from freqlabs@FreeBSD.org) Received: (from freqlabs@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 038852Hw017135; Wed, 8 Apr 2020 08:05:02 GMT (envelope-from freqlabs@FreeBSD.org) Message-Id: <202004080805.038852Hw017135@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: freqlabs set sender to freqlabs@FreeBSD.org using -f From: Ryan Moeller Date: Wed, 8 Apr 2020 08:05:02 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r359722 - in stable/11: cddl/contrib/opensolaris/lib/libzfs/common sys/cddl/contrib/opensolaris/uts/common/fs/zfs sys/cddl/contrib/opensolaris/uts/common/sys/fs X-SVN-Group: stable-11 X-SVN-Commit-Author: freqlabs X-SVN-Commit-Paths: in stable/11: cddl/contrib/opensolaris/lib/libzfs/common sys/cddl/contrib/opensolaris/uts/common/fs/zfs sys/cddl/contrib/opensolaris/uts/common/sys/fs X-SVN-Commit-Revision: 359722 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-11@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for only the 11-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Apr 2020 08:05:04 -0000 Author: freqlabs Date: Wed Apr 8 08:05:02 2020 New Revision: 359722 URL: https://svnweb.freebsd.org/changeset/base/359722 Log: MFC r359303 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) openzfs/zfs@d8d418ff0cc90776182534bce10b01e9487b63e4 Modified: stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_util.c stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_objset.c stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_send.c stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dir.c stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c stable/11/sys/cddl/contrib/opensolaris/uts/common/sys/fs/zfs.h Directory Properties: stable/11/ (props changed) Modified: stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h ============================================================================== --- stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h Wed Apr 8 06:06:13 2020 (r359721) +++ stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs.h Wed Apr 8 08:05:02 2020 (r359722) @@ -140,6 +140,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; Modified: stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c ============================================================================== --- stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c Wed Apr 8 06:06:13 2020 (r359721) +++ stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_dataset.c Wed Apr 8 08:05:02 2020 (r359722) @@ -3616,11 +3616,6 @@ zfs_create(libzfs_handle_t *hdl, const char *path, zfs "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 " Modified: stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c ============================================================================== --- stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c Wed Apr 8 06:06:13 2020 (r359721) +++ stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c Wed Apr 8 08:05:02 2020 (r359722) @@ -28,6 +28,8 @@ * 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. */ #include @@ -3356,6 +3358,7 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const * - 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, @@ -3373,6 +3376,25 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const 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, @@ -3422,6 +3444,8 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const 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 @@ -3448,6 +3472,21 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const zcmd_free_nvlists(&zc); 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; } Modified: stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_util.c ============================================================================== --- stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_util.c Wed Apr 8 06:06:13 2020 (r359721) +++ stable/11/cddl/contrib/opensolaris/lib/libzfs/common/libzfs_util.c Wed Apr 8 08:05:02 2020 (r359722) @@ -261,6 +261,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: @@ -526,6 +528,9 @@ zpool_standard_error_fmt(libzfs_handle_t *hdl, int err break; 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)); Modified: stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_objset.c ============================================================================== --- stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_objset.c Wed Apr 8 06:06:13 2020 (r359721) +++ stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_objset.c Wed Apr 8 08:05:02 2020 (r359722) @@ -28,6 +28,7 @@ * Copyright (c) 2015, STRATO AG, Inc. All rights reserved. * Copyright (c) 2014 Integros [integros.com] * Copyright 2017 Nexenta Systems, Inc. + * Copyright (c) 2018, loli10K . All rights reserved. */ /* Portions Copyright 2010 Robert Milkowski */ @@ -905,6 +906,8 @@ dmu_objset_create_check(void *arg, dmu_tx_t *tx) dmu_objset_create_arg_t *doca = arg; dsl_pool_t *dp = dmu_tx_pool(tx); dsl_dir_t *pdd; + dsl_dataset_t *parentds; + objset_t *parentos; const char *tail; int error; @@ -926,6 +929,30 @@ dmu_objset_create_check(void *arg, dmu_tx_t *tx) } error = dsl_fs_ss_limit_check(pdd, 1, ZFS_PROP_FILESYSTEM_LIMIT, NULL, doca->doca_cred); + if (error != 0) { + dsl_dir_rele(pdd, FTAG); + return (error); + } + + /* can't create below anything but filesystems (eg. no ZVOLs) */ + error = dsl_dataset_hold_obj(pdd->dd_pool, + dsl_dir_phys(pdd)->dd_head_dataset_obj, FTAG, &parentds); + if (error != 0) { + dsl_dir_rele(pdd, FTAG); + return (error); + } + error = dmu_objset_from_ds(parentds, &parentos); + if (error != 0) { + dsl_dataset_rele(parentds, FTAG); + dsl_dir_rele(pdd, FTAG); + return (error); + } + if (dmu_objset_type(parentos) != DMU_OST_ZFS) { + dsl_dataset_rele(parentds, FTAG); + dsl_dir_rele(pdd, FTAG); + return (SET_ERROR(ZFS_ERR_WRONG_PARENT)); + } + dsl_dataset_rele(parentds, FTAG); dsl_dir_rele(pdd, FTAG); return (error); Modified: stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_send.c ============================================================================== --- stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_send.c Wed Apr 8 06:06:13 2020 (r359721) +++ stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_send.c Wed Apr 8 08:05:02 2020 (r359722) @@ -27,6 +27,7 @@ * Copyright 2014 HybridCluster. All rights reserved. * Copyright 2016 RackTop Systems. * Copyright (c) 2014 Integros [integros.com] + * Copyright (c) 2018, loli10K . All rights reserved. */ #include @@ -1305,6 +1306,7 @@ recv_begin_check_existing_impl(dmu_recv_begin_arg_t *d uint64_t fromguid) { uint64_t val; + uint64_t children; int error; dsl_pool_t *dp = ds->ds_dir->dd_pool; @@ -1326,6 +1328,15 @@ recv_begin_check_existing_impl(dmu_recv_begin_arg_t *d if (error != ENOENT) return (error == 0 ? SET_ERROR(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(ZFS_ERR_WRONG_PARENT)); + /* * Check snapshot limit before receiving. We'll recheck again at the * end, but might as well abort before receiving if we're already over @@ -1459,6 +1470,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 @@ -1503,6 +1515,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(ds, FTAG); + return (error); + } + if (dmu_objset_type(os) != DMU_OST_ZFS) { + dsl_dataset_rele(ds, FTAG); + return (SET_ERROR(ZFS_ERR_WRONG_PARENT)); + } + if (drba->drba_origin != NULL) { dsl_dataset_t *origin; error = dsl_dataset_hold(dp, drba->drba_origin, @@ -1524,6 +1547,7 @@ dmu_recv_begin_check(void *arg, dmu_tx_t *tx) } dsl_dataset_rele(origin, FTAG); } + dsl_dataset_rele(ds, FTAG); error = 0; } Modified: stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dir.c ============================================================================== --- stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dir.c Wed Apr 8 06:06:13 2020 (r359721) +++ stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dsl_dir.c Wed Apr 8 08:05:02 2020 (r359722) @@ -26,6 +26,7 @@ * Copyright (c) 2014 Joyent, Inc. All rights reserved. * Copyright (c) 2014 Spectra Logic Corporation, All rights reserved. * Copyright 2015 Nexenta Systems, Inc. All rights reserved. + * Copyright (c) 2018, loli10K . All rights reserved. */ #include @@ -1852,6 +1853,8 @@ dsl_dir_rename_check(void *arg, dmu_tx_t *tx) dsl_pool_t *dp = dmu_tx_pool(tx); dsl_dir_t *dd, *newparent; dsl_valid_rename_arg_t dvra; + dsl_dataset_t *parentds; + objset_t *parentos; const char *mynewname; int error; @@ -1881,6 +1884,29 @@ dsl_dir_rename_check(void *arg, dmu_tx_t *tx) dsl_dir_rele(dd, FTAG); return (SET_ERROR(EEXIST)); } + + /* can't rename below anything but filesystems (eg. no ZVOLs) */ + error = dsl_dataset_hold_obj(newparent->dd_pool, + dsl_dir_phys(newparent)->dd_head_dataset_obj, FTAG, &parentds); + if (error != 0) { + dsl_dir_rele(newparent, FTAG); + dsl_dir_rele(dd, FTAG); + return (error); + } + error = dmu_objset_from_ds(parentds, &parentos); + if (error != 0) { + dsl_dataset_rele(parentds, FTAG); + dsl_dir_rele(newparent, FTAG); + dsl_dir_rele(dd, FTAG); + return (error); + } + if (dmu_objset_type(parentos) != DMU_OST_ZFS) { + dsl_dataset_rele(parentds, FTAG); + dsl_dir_rele(newparent, FTAG); + dsl_dir_rele(dd, FTAG); + return (error); + } + dsl_dataset_rele(parentds, FTAG); ASSERT3U(strnlen(ddra->ddra_newname, ZFS_MAX_DATASET_NAME_LEN), <, ZFS_MAX_DATASET_NAME_LEN); Modified: stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c ============================================================================== --- stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c Wed Apr 8 06:06:13 2020 (r359721) +++ stable/11/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c Wed Apr 8 08:05:02 2020 (r359722) @@ -33,8 +33,8 @@ * Copyright (c) 2014 Integros [integros.com] * Copyright 2016 Toomas Soome * Copyright 2017 RackTop Systems. - * Copyright (c) 2017 Datto Inc. - * Copyright 2016 Toomas Soome + * Copyright (c) 2018, loli10K . All rights reserved. + * Copyright (c) 2019 Datto Inc. */ /* @@ -3131,8 +3131,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(ZFS_ERR_WRONG_PARENT)); /* * Pull out creator prop choices, if any. @@ -3208,15 +3209,11 @@ zfs_fill_zplprops(const char *dataset, nvlist_t *creat 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); Modified: stable/11/sys/cddl/contrib/opensolaris/uts/common/sys/fs/zfs.h ============================================================================== --- stable/11/sys/cddl/contrib/opensolaris/uts/common/sys/fs/zfs.h Wed Apr 8 06:06:13 2020 (r359721) +++ stable/11/sys/cddl/contrib/opensolaris/uts/common/sys/fs/zfs.h Wed Apr 8 08:05:02 2020 (r359722) @@ -1020,7 +1020,8 @@ typedef enum { ZFS_ERR_DISCARDING_CHECKPOINT, ZFS_ERR_NO_CHECKPOINT, ZFS_ERR_DEVRM_IN_PROGRESS, - ZFS_ERR_VDEV_TOO_BIG + ZFS_ERR_VDEV_TOO_BIG, + ZFS_ERR_WRONG_PARENT, } zfs_errno_t; /*