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 718ceea50c0e6deefe39ac4ff6a8c0a78ee4d9ba..cb416b9d3a9d007756fa589163a090e8d08e9765 100644 (file)
--- 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 385183122e43e6510184930e2ebf71e1a862e75c..4a11837292c7ce82ebf5f71a273d0d4e5ad882b4 100755 (executable)
--- 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."