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 <behlendorf1@llnl.gov>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Closes #10919

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

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);
 }
 

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 {

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]

tests/zfs-tests/cmd/Makefile.am

@@ -1,6 +1,7 @@
 EXTRA_DIST = file_common.h
 
 SUBDIRS = \
+	badsend \
 	btree_test \
 	chg_usr_exec \
 	devname2devid \

tests/zfs-tests/cmd/badsend/.gitignore

@@ -0,0 +1 @@
+/badsend

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

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);
+}

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

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
 

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."