]> git.proxmox.com Git - mirror_zfs.git/commitdiff
Drop references when skipping dmu_send due to EXDEV
authorRyan Moeller <ryan@iXsystems.com>
Wed, 30 Sep 2020 20:19:49 +0000 (16:19 -0400)
committerBrian Behlendorf <behlendorf1@llnl.gov>
Thu, 1 Oct 2020 19:22:36 +0000 (12:22 -0700)
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
module/os/freebsd/zfs/kmod_core.c
module/zfs/dmu_send.c
tests/runfiles/common.run
tests/zfs-tests/cmd/Makefile.am
tests/zfs-tests/cmd/badsend/.gitignore [new file with mode: 0644]
tests/zfs-tests/cmd/badsend/Makefile.am [new file with mode: 0644]
tests/zfs-tests/cmd/badsend/badsend.c [new file with mode: 0644]
tests/zfs-tests/include/commands.cfg
tests/zfs-tests/tests/functional/rsend/Makefile.am
tests/zfs-tests/tests/functional/rsend/send_invalid.ksh [new file with mode: 0755]

index f149ab6d1b83e875c9185310ef2b6f4575e825ff..a1664151bc9ad61b6d5e2783f6c50d878db66fc1 100644 (file)
@@ -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
index 4c696129857a6945fd3c42d4a0ec95c080ddbdca..3a13271aac6fc151e962b126418fb6cb549168e8 100644 (file)
@@ -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);
 }
 
index 07c2d6ef248a7518950235751f546ee84a884e96..9480c8b754974a289ad2e1150bb618fb45454c37 100644 (file)
@@ -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 {
index 725afe2f054af33f406ac4c7ec999efc92e3bf21..e06281648e7011b0473b46fec79590ec0d22d726 100644 (file)
@@ -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]
index 20e85cf6bb3fa2738fcd0ce7afd60f4f1740c7e5..bf54c1d4571069c9ddf057cfea3d548f465b26e7 100644 (file)
@@ -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 (file)
index 0000000..d2efa62
--- /dev/null
@@ -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 (file)
index 0000000..5a8946f
--- /dev/null
@@ -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 (file)
index 0000000..af17bc7
--- /dev/null
@@ -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);
+}
index 4c11bf146378b626358737db0306f12cf53a26e3..5a507b94ab6c6732fcb7c48f5b191f83657f938d 100644 (file)
@@ -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
index cf6c727dfa16536cc243f11139bef5c64fef8afc..61be2ec1889db596728fb833a533c0e7c0dd22d2 100644 (file)
@@ -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 (executable)
index 0000000..a0abe64
--- /dev/null
@@ -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."