From: LOLi <loli10K@users.noreply.github.com>
Date: Mon, 26 Feb 2018 17:55:18 +0000 (+0100)
Subject: Fix segfault in zfs_do_bookmark()
X-Git-Tag: zfs-0.8.0-rc1~308
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4af6873af6f8e9f6355e4962262c06d449664d26;p=zfs

Fix segfault in zfs_do_bookmark()

When invoked with wrong parameters 'zfs bookmark' fails to gracefully
validate user input and crashes.

This is a regression accidentally introduced in 587e228; this commit
adds additional tests to the ZFS Test Suite to exercise this codepath.

Reviewed-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: KireinaHoro <i@jsteward.moe>
Signed-off-by:  loli10K <ezomori.nozomu@gmail.com>
Closes #7228
Closes #7229
---

diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c
index 718ceea50..cb416b9d3 100644
--- a/cmd/zfs/zfs_main.c
+++ b/cmd/zfs/zfs_main.c
@@ -7112,6 +7112,12 @@ zfs_do_bookmark(int argc, char **argv)
 		goto usage;
 	}
 
+	if (strchr(argv[0], '@') == NULL) {
+		(void) fprintf(stderr,
+		    gettext("invalid snapshot name '%s': "
+		    "must contain a '@'\n"), argv[0]);
+		goto usage;
+	}
 	if (strchr(argv[1], '#') == NULL) {
 		(void) fprintf(stderr,
 		    gettext("invalid bookmark name '%s': "
diff --git a/tests/zfs-tests/tests/functional/cli_root/zfs_bookmark/zfs_bookmark_cliargs.ksh b/tests/zfs-tests/tests/functional/cli_root/zfs_bookmark/zfs_bookmark_cliargs.ksh
index 385183122..4a1183729 100755
--- a/tests/zfs-tests/tests/functional/cli_root/zfs_bookmark/zfs_bookmark_cliargs.ksh
+++ b/tests/zfs-tests/tests/functional/cli_root/zfs_bookmark/zfs_bookmark_cliargs.ksh
@@ -35,6 +35,8 @@
 # 2. Verify we can create a bookmark specifying snapshot and bookmark full paths
 # 3. Verify we can create a bookmark specifying the snapshot name
 # 4. Verify we can create a bookmark specifying the bookmark name
+# 5. Verify at least a full dataset path is required and both snapshot and
+#    bookmark name must be valid
 #
 
 verify_runnable "both"
@@ -49,7 +51,7 @@ function cleanup
 	fi
 }
 
-log_assert "'zfs bookmark' works as expected when passed valid arguments."
+log_assert "'zfs bookmark' should work only when passed valid arguments."
 log_onexit cleanup
 
 DATASET="$TESTPOOL/$TESTFS"
@@ -74,4 +76,25 @@ log_must zfs bookmark "$DATASET@$TESTSNAP" "#$TESTBM"
 log_must eval "bkmarkexists $DATASET#$TESTBM"
 log_must zfs destroy "$DATASET#$TESTBM"
 
-log_pass "'zfs bookmark' works as expected when passed valid arguments."
+# Verify at least a full dataset path is required and both snapshot and
+# bookmark name must be valid
+log_mustnot zfs bookmark "@$TESTSNAP" "#$TESTBM"
+log_mustnot zfs bookmark "$TESTSNAP" "#$TESTBM"
+log_mustnot zfs bookmark "@$TESTSNAP" "$TESTBM"
+log_mustnot zfs bookmark "$TESTSNAP" "$TESTBM"
+log_mustnot zfs bookmark "$TESTSNAP" "$DATASET#$TESTBM"
+log_mustnot zfs bookmark "$DATASET" "$TESTBM"
+log_mustnot zfs bookmark "$DATASET@" "$TESTBM"
+log_mustnot zfs bookmark "$DATASET" "#$TESTBM"
+log_mustnot zfs bookmark "$DATASET@" "#$TESTBM"
+log_mustnot zfs bookmark "$DATASET@$TESTSNAP" "$TESTBM"
+log_mustnot zfs bookmark "@" "#$TESTBM"
+log_mustnot zfs bookmark "@" "#"
+log_mustnot zfs bookmark "@$TESTSNAP" "#"
+log_mustnot zfs bookmark "@$TESTSNAP" "$DATASET#"
+log_mustnot zfs bookmark "@$TESTSNAP" "$DATASET"
+log_mustnot zfs bookmark "$TESTSNAP" "$DATASET#"
+log_mustnot zfs bookmark "$TESTSNAP" "$DATASET"
+log_mustnot eval "bkmarkexists $DATASET#$TESTBM"
+
+log_pass "'zfs bookmark' works as expected only when passed valid arguments."