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 <[email protected]>
Signed-off-by: Ryan Moeller <[email protected]>
Closes #10919 

11 files changed, 216 insertions, 7 deletions

diff --git a/configure.ac b/configure.ac
index 471fcfcec..8a30ac6b9 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 <sys/zio_checksum.h>
 #include <sys/vdev_removal.h>
 #include <sys/dsl_crypt.h>
+#include <sys/zfs_context.h>
 
 #include <sys/zfs_ioctl_compat.h>
 #include <sys/zfs_ioctl_impl.h>
@@ -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 23bcf21d2..a29578b36 100644
--- a/tests/runfiles/common.run
+++ b/tests/runfiles/common.run
@@ -784,7 +784,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 <libzfs.h>
+#include <libzfs_core.h>
+
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sysexits.h>
+#include <err.h>
+
+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."