Avoid core dump on invalid redaction bookmark

libzfs aborts and dumps core on EINVAL from the kernel when trying to do a redacted send with a bookmark that is not a redaction bookmark. Check if the bookmark given for redactions is actually a redaction bookmark. Print an error message and exit gracefully if it is not. Signed-off-by: Ryan Moeller <[email protected]>
openzfs · Mar 16, 2020 · 470a20f · 470a20f
1 parent 4d32aba
commit 470a20f
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 4 deletions.
diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c
@@ -4409,7 +4409,13 @@ zfs_do_send(int argc, char **argv)
 		char frombuf[ZFS_MAX_DATASET_NAME_LEN];
 
 		if (redactbook != NULL) {
-			if (strchr(argv[0], '@') == NULL) {
+			char bookname[ZFS_MAX_DATASET_NAME_LEN];
+			nvlist_t *redact_snaps;
+			zfs_handle_t *book_zhp;
+			char *snapname = argv[0];
+			char *at = strchr(snapname, '@');
+			int dsnamelen;
+			if (at == NULL) {
 				(void) fprintf(stderr, gettext("Error: Cannot "
 				    "do a redacted send to a filesystem.\n"));
 				return (1);
@@ -4420,6 +4426,26 @@ zfs_do_send(int argc, char **argv)
 				    "not contain '#'\n"));
 				return (1);
 			}
+			dsnamelen = at - snapname;
+			if (snprintf(bookname, sizeof (bookname), "%.*s#%s",
+			    dsnamelen, snapname, redactbook)
+			    >= sizeof (bookname)) {
+				(void) fprintf(stderr, gettext("Error: "
+				    "invalid bookmark name.\n"));
+				return (1);
+			}
+			book_zhp = zfs_open(g_zfs, bookname, ZFS_TYPE_BOOKMARK);
+			if (book_zhp == NULL)
+				return (1);
+			if (nvlist_lookup_nvlist(book_zhp->zfs_props,
+			    zfs_prop_to_name(ZFS_PROP_REDACT_SNAPS),
+			    &redact_snaps) != 0 || redact_snaps == NULL) {
+				zfs_close(book_zhp);
+				(void) fprintf(stderr, gettext("Error: "
+				    "not a redaction bookmark.\n"));
+				return (1);
+			}
+			zfs_close(book_zhp);
 		}
 
 		zhp = zfs_open(g_zfs, argv[0], ZFS_TYPE_DATASET);

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
@@ -222,12 +222,12 @@ log_must zfs redact "$DATASET@$TESTSNAP" "$TESTBM" "$DATASET@$TESTSNAP2"
 log_must eval "zfs get all $DATASET#$TESTBM | grep redact_snaps"
 ## copy the redaction bookmark
 log_must zfs bookmark "$DATASET#$TESTBM" "#$TESTBMCOPY"
-log_must eval "zfs send --redact "$TESTBMCOPY" -i $DATASET@$TESTSNAP $DATASET@$TESTSNAP2 2>&1 | head -n 100 | grep 'internal error: Invalid argument'"
 log_mustnot eval "zfs get all $DATASET#$TESTBMCOPY | grep redact_snaps"
+log_must eval "zfs send --redact "$TESTBMCOPY" -i $DATASET@$TESTSNAP $DATASET@$TESTSNAP2 2>&1 | head -n 100 | grep 'not a redaction bookmark'"
 # try the above again after destroying the source bookmark, preventive measure for future work
 log_must zfs destroy "$DATASET#$TESTBM"
-log_must eval "zfs send --redact "$TESTBMCOPY" -i $DATASET@$TESTSNAP $DATASET@$TESTSNAP2 2>&1 | head -n 100 | grep 'internal error: Invalid argument'"
 log_mustnot eval "zfs get all $DATASET#$TESTBMCOPY | grep redact_snaps"
+log_must eval "zfs send --redact "$TESTBMCOPY" -i $DATASET@$TESTSNAP $DATASET@$TESTSNAP2 2>&1 | head -n 100 | grep 'not a redaction bookmark'"
 ## cleanup
 log_must eval "destroy_dataset $DATASET@$TESTSNAP2"
 log_must zfs destroy "$DATASET#$TESTBMCOPY"

diff --git a/tests/zfs-tests/tests/functional/redacted_send/redacted_negative.ksh b/tests/zfs-tests/tests/functional/redacted_send/redacted_negative.ksh
@@ -79,7 +79,7 @@ log_mustnot zfs redact testpool2/recvfs@snap2 book7 testpool2/recvfs@snap
 
 # Non-redaction bookmark cannot be sent and produces invalid argument error
 log_must zfs bookmark "$sendfs@snap1" "$sendfs#book8"
-log_must eval "zfs send --redact book8 -i $sendfs@snap1 $sendfs@snap2 2>&1 | head -n 100 | grep 'internal error: Invalid argument'"
+log_must eval "zfs send --redact book8 -i $sendfs@snap1 $sendfs@snap2 2>&1 | head -n 100 | grep 'not a redaction bookmark'"
 
 # Error messages for common usage errors
 log_mustnot_expect "not contain '#'"    zfs redact $sendfs@snap1 \#book $sendfs@snap2