From e58dee8cae11beb82fce2bb2cb039584b82b4040 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Wed, 30 Sep 2020 16:19:49 -0400 Subject: [PATCH] Drop references when skipping dmu_send due to EXDEV When an invalid incremental send is requested where the "to" ds is before the "from" ds, make sure to drop the reference to the pool and the dataset before returning the error. Add an assert on FreeBSD to make sure we don't hold any locks after returning from an ioctl. Add some test coverage. Reviewed-by: Brian Behlendorf Signed-off-by: Ryan Moeller Closes #10919 --- configure.ac | 1 + module/os/freebsd/zfs/kmod_core.c | 4 +- module/zfs/dmu_send.c | 11 +- tests/runfiles/common.run | 2 +- tests/zfs-tests/cmd/Makefile.am | 1 + tests/zfs-tests/cmd/badsend/.gitignore | 1 + tests/zfs-tests/cmd/badsend/Makefile.am | 11 ++ tests/zfs-tests/cmd/badsend/badsend.c | 136 ++++++++++++++++++ tests/zfs-tests/include/commands.cfg | 3 +- .../tests/functional/rsend/Makefile.am | 1 + .../tests/functional/rsend/send_invalid.ksh | 52 +++++++ 11 files changed, 216 insertions(+), 7 deletions(-) create mode 100644 tests/zfs-tests/cmd/badsend/.gitignore create mode 100644 tests/zfs-tests/cmd/badsend/Makefile.am create mode 100644 tests/zfs-tests/cmd/badsend/badsend.c create mode 100755 tests/zfs-tests/tests/functional/rsend/send_invalid.ksh diff --git a/configure.ac b/configure.ac index f149ab6d1..a1664151b 100644 --- a/configure.ac +++ b/configure.ac @@ -204,6 +204,7 @@ AC_CONFIG_FILES([ tests/zfs-tests/Makefile tests/zfs-tests/callbacks/Makefile tests/zfs-tests/cmd/Makefile + tests/zfs-tests/cmd/badsend/Makefile tests/zfs-tests/cmd/btree_test/Makefile tests/zfs-tests/cmd/chg_usr_exec/Makefile tests/zfs-tests/cmd/devname2devid/Makefile diff --git a/module/os/freebsd/zfs/kmod_core.c b/module/os/freebsd/zfs/kmod_core.c index 4c6961298..3a13271aa 100644 --- a/module/os/freebsd/zfs/kmod_core.c +++ b/module/os/freebsd/zfs/kmod_core.c @@ -86,6 +86,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include @@ -98,7 +99,7 @@ __FBSDID("$FreeBSD$"); SYSCTL_DECL(_vfs_zfs); SYSCTL_DECL(_vfs_zfs_vdev); - +extern uint_t rrw_tsd_key; static int zfs_version_ioctl = ZFS_IOCVER_OZFS; SYSCTL_DECL(_vfs_zfs_version); SYSCTL_INT(_vfs_zfs_version, OID_AUTO, ioctl, CTLFLAG_RD, &zfs_version_ioctl, @@ -180,6 +181,7 @@ out: if (zcl) kmem_free(zcl, sizeof (zfs_cmd_legacy_t)); kmem_free(zc, sizeof (zfs_cmd_t)); + MPASS(tsd_get(rrw_tsd_key) == NULL); return (error); } diff --git a/module/zfs/dmu_send.c b/module/zfs/dmu_send.c index 07c2d6ef2..9480c8b75 100644 --- a/module/zfs/dmu_send.c +++ b/module/zfs/dmu_send.c @@ -2684,12 +2684,15 @@ dmu_send_obj(const char *pool, uint64_t tosnap, uint64_t fromsnap, bcopy(fromredact, dspp.fromredactsnaps, size); } - if (!dsl_dataset_is_before(dspp.to_ds, fromds, 0)) { + boolean_t is_before = + dsl_dataset_is_before(dspp.to_ds, fromds, 0); + dspp.is_clone = (dspp.to_ds->ds_dir != + fromds->ds_dir); + dsl_dataset_rele(fromds, FTAG); + if (!is_before) { + dsl_pool_rele(dspp.dp, FTAG); err = SET_ERROR(EXDEV); } else { - dspp.is_clone = (dspp.to_ds->ds_dir != - fromds->ds_dir); - dsl_dataset_rele(fromds, FTAG); err = dmu_send_impl(&dspp); } } else { diff --git a/tests/runfiles/common.run b/tests/runfiles/common.run index 725afe2f0..e06281648 100644 --- a/tests/runfiles/common.run +++ b/tests/runfiles/common.run @@ -790,7 +790,7 @@ tests = ['recv_dedup', 'recv_dedup_encrypted_zvol', 'rsend_001_pos', 'send_freeobjects', 'send_realloc_files', 'send_realloc_encrypted_files', 'send_spill_block', 'send_holds', 'send_hole_birth', 'send_mixed_raw', 'send-wR_encrypted_zvol', - 'send_partial_dataset'] + 'send_partial_dataset', 'send_invalid'] tags = ['functional', 'rsend'] [tests/functional/scrub_mirror] diff --git a/tests/zfs-tests/cmd/Makefile.am b/tests/zfs-tests/cmd/Makefile.am index 20e85cf6b..bf54c1d45 100644 --- a/tests/zfs-tests/cmd/Makefile.am +++ b/tests/zfs-tests/cmd/Makefile.am @@ -1,6 +1,7 @@ EXTRA_DIST = file_common.h SUBDIRS = \ + badsend \ btree_test \ chg_usr_exec \ devname2devid \ diff --git a/tests/zfs-tests/cmd/badsend/.gitignore b/tests/zfs-tests/cmd/badsend/.gitignore new file mode 100644 index 000000000..d2efa627a --- /dev/null +++ b/tests/zfs-tests/cmd/badsend/.gitignore @@ -0,0 +1 @@ +/badsend diff --git a/tests/zfs-tests/cmd/badsend/Makefile.am b/tests/zfs-tests/cmd/badsend/Makefile.am new file mode 100644 index 000000000..5a8946f0d --- /dev/null +++ b/tests/zfs-tests/cmd/badsend/Makefile.am @@ -0,0 +1,11 @@ +include $(top_srcdir)/config/Rules.am + +pkgexecdir = $(datadir)/@PACKAGE@/zfs-tests/bin + +pkgexec_PROGRAMS = badsend + +badsend_SOURCES = badsend.c +badsend_LDADD = \ + $(abs_top_builddir)/lib/libzfs_core/libzfs_core.la \ + $(abs_top_builddir)/lib/libzfs/libzfs.la \ + $(abs_top_builddir)/lib/libnvpair/libnvpair.la diff --git a/tests/zfs-tests/cmd/badsend/badsend.c b/tests/zfs-tests/cmd/badsend/badsend.c new file mode 100644 index 000000000..af17bc725 --- /dev/null +++ b/tests/zfs-tests/cmd/badsend/badsend.c @@ -0,0 +1,136 @@ +/* + * CDDL HEADER START + * + * The contents of this file are subject to the terms of the + * Common Development and Distribution License (the "License"). + * You may not use this file except in compliance with the License. + * + * You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE + * or http://www.opensolaris.org/os/licensing. + * See the License for the specific language governing permissions + * and limitations under the License. + * + * When distributing Covered Code, include this CDDL HEADER in each + * file and include the License file at usr/src/OPENSOLARIS.LICENSE. + * If applicable, add the following below this CDDL HEADER, with the + * fields enclosed by brackets "[]" replaced with your own identifying + * information: Portions Copyright [yyyy] [name of copyright owner] + * + * CDDL HEADER END + */ + +/* + * Portions Copyright 2020 iXsystems, Inc. + */ + +/* + * Test some invalid send operations with libzfs/libzfs_core. + * + * Specifying the to and from snaps in the wrong order should return EXDEV. + * We are checking that the early return doesn't accidentally leave any + * references held, so this test is designed to trigger a panic when asserts + * are verified with the bug present. + */ + +#include +#include + +#include +#include +#include +#include +#include +#include + +static void +usage(const char *name) +{ + fprintf(stderr, "usage: %s snap0 snap1\n", name); + exit(EX_USAGE); +} + +int +main(int argc, char const * const argv[]) +{ + sendflags_t flags = { 0 }; + libzfs_handle_t *zhdl; + zfs_handle_t *zhp; + const char *fromfull, *tofull, *fsname, *fromsnap, *tosnap, *p; + uint64_t size; + int fd, error; + + if (argc != 3) + usage(argv[0]); + + fromfull = argv[1]; + tofull = argv[2]; + + p = strchr(fromfull, '@'); + if (p == NULL) + usage(argv[0]); + fromsnap = p + 1; + + p = strchr(tofull, '@'); + if (p == NULL) + usage(argv[0]); + tosnap = p + 1; + + fsname = strndup(tofull, p - tofull); + if (strncmp(fsname, fromfull, p - tofull) != 0) + usage(argv[0]); + + fd = open("/dev/null", O_WRONLY); + if (fd == -1) + err(EX_OSERR, "open(\"/dev/null\", O_WRONLY)"); + + zhdl = libzfs_init(); + if (zhdl == NULL) + errx(EX_OSERR, "libzfs_init(): %s", libzfs_error_init(errno)); + + zhp = zfs_open(zhdl, fsname, ZFS_TYPE_FILESYSTEM); + if (zhp == NULL) + err(EX_OSERR, "zfs_open(\"%s\")", fsname); + + /* + * Exercise EXDEV in dmu_send_obj. The error gets translated to + * EZFS_CROSSTARGET in libzfs. + */ + error = zfs_send(zhp, tosnap, fromsnap, &flags, fd, NULL, NULL, NULL); + if (error == 0 || libzfs_errno(zhdl) != EZFS_CROSSTARGET) + errx(EX_OSERR, "zfs_send(\"%s\", \"%s\") should have failed " + "with EZFS_CROSSTARGET, not %d", + tofull, fromfull, libzfs_errno(zhdl)); + printf("zfs_send(\"%s\", \"%s\"): %s\n", + tofull, fromfull, libzfs_error_description(zhdl)); + + zfs_close(zhp); + + /* + * Exercise EXDEV in dmu_send. + */ + error = lzc_send_resume_redacted(fromfull, tofull, fd, 0, 0, 0, NULL); + if (error != EXDEV) + errx(EX_OSERR, "lzc_send_resume_redacted(\"%s\", \"%s\")" + " should have failed with EXDEV, not %d", + fromfull, tofull, error); + printf("lzc_send_resume_redacted(\"%s\", \"%s\"): %s\n", + fromfull, tofull, strerror(error)); + + /* + * Exercise EXDEV in dmu_send_estimate_fast. + */ + error = lzc_send_space_resume_redacted(fromfull, tofull, 0, 0, 0, 0, + NULL, fd, &size); + if (error != EXDEV) + errx(EX_OSERR, "lzc_send_space_resume_redacted(\"%s\", \"%s\")" + " should have failed with EXDEV, not %d", + fromfull, tofull, error); + printf("lzc_send_space_resume_redacted(\"%s\", \"%s\"): %s\n", + fromfull, tofull, strerror(error)); + + close(fd); + libzfs_fini(zhdl); + free((void *)fsname); + + return (EXIT_SUCCESS); +} diff --git a/tests/zfs-tests/include/commands.cfg b/tests/zfs-tests/include/commands.cfg index 4c11bf146..5a507b94a 100644 --- a/tests/zfs-tests/include/commands.cfg +++ b/tests/zfs-tests/include/commands.cfg @@ -190,7 +190,8 @@ export ZFS_FILES='zdb zstreamdump zfs_ids_to_path' -export ZFSTEST_FILES='btree_test +export ZFSTEST_FILES='badsend + btree_test chg_usr_exec devname2devid dir_rd_update diff --git a/tests/zfs-tests/tests/functional/rsend/Makefile.am b/tests/zfs-tests/tests/functional/rsend/Makefile.am index cf6c727df..61be2ec18 100644 --- a/tests/zfs-tests/tests/functional/rsend/Makefile.am +++ b/tests/zfs-tests/tests/functional/rsend/Makefile.am @@ -51,6 +51,7 @@ dist_pkgdata_SCRIPTS = \ send_spill_block.ksh \ send_holds.ksh \ send_hole_birth.ksh \ + send_invalid.ksh \ send_mixed_raw.ksh \ send-wR_encrypted_zvol.ksh diff --git a/tests/zfs-tests/tests/functional/rsend/send_invalid.ksh b/tests/zfs-tests/tests/functional/rsend/send_invalid.ksh new file mode 100755 index 000000000..a0abe64b4 --- /dev/null +++ b/tests/zfs-tests/tests/functional/rsend/send_invalid.ksh @@ -0,0 +1,52 @@ +#!/bin/ksh + +# +# This file and its contents are supplied under the terms of the +# Common Development and Distribution License ("CDDL"), version a.0. +# You may only use this file in accordance with the terms of version +# a.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. +# + +# +# Portions Copyright 2020 iXsystems, Inc. +# + +. $STF_SUITE/include/libtest.shlib +. $STF_SUITE/tests/functional/rsend/rsend.kshlib + +# +# Description: +# Verify that send with invalid options will fail gracefully. +# +# Strategy: +# 1. Perform zfs send on the cli with the order of the snapshots reversed +# 2. Perform zfs send using libzfs with the order of the snapshots reversed +# + +verify_runnable "both" + +log_assert "Verify that send with invalid options will fail gracefully." + +function cleanup +{ + datasetexists $testfs && destroy_dataset $testfs -r +} +log_onexit cleanup + +testfs=$POOL/fs + +log_must zfs create $testfs +log_must zfs snap $testfs@snap0 +log_must zfs snap $testfs@snap1 + +# Test bad send with the CLI +log_mustnot eval "zfs send -i $testfs@snap1 $testfs@snap0 >/dev/null" + +# Test bad send with libzfs/libzfs_core +log_must badsend $testfs@snap0 $testfs@snap1 + +log_pass "Send with invalid options fails gracefully."