From 22df2457a7b8265d999a2fe48f3e8e941c8fdde5 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Wed, 18 Mar 2020 15:54:12 -0400 Subject: [PATCH] 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. Move redacted bookmark validation into libzfs. Check if the bookmark given for redactions is actually a redaction bookmark. Print an error message and exit gracefully if it is not. Don't abort on EINVAL in zfs_send_one. Reviewed-by: Paul Dagnelie Reviewed-by: Brian Behlendorf Signed-off-by: Ryan Moeller Closes #10138 --- cmd/zfs/zfs_main.c | 22 ++------- lib/libzfs/libzfs_sendrecv.c | 48 +++++++++++++++++-- .../zfs_bookmark/zfs_bookmark_cliargs.ksh | 4 +- .../redacted_send/redacted_negative.ksh | 2 +- 4 files changed, 51 insertions(+), 25 deletions(-) diff --git a/cmd/zfs/zfs_main.c b/cmd/zfs/zfs_main.c index 82b91754e8..150bebd323 100644 --- a/cmd/zfs/zfs_main.c +++ b/cmd/zfs/zfs_main.c @@ -4408,24 +4408,6 @@ zfs_do_send(int argc, char **argv) if (!(flags.replicate || flags.doall)) { char frombuf[ZFS_MAX_DATASET_NAME_LEN]; - if (redactbook != NULL) { - if (strchr(argv[0], '@') == NULL) { - (void) fprintf(stderr, gettext("Error: Cannot " - "do a redacted send to a filesystem.\n")); - return (1); - } - if (strchr(redactbook, '#') != NULL) { - (void) fprintf(stderr, gettext("Error: " - "redaction bookmark argument must " - "not contain '#'\n")); - return (1); - } - } - - zhp = zfs_open(g_zfs, argv[0], ZFS_TYPE_DATASET); - if (zhp == NULL) - return (1); - if (fromname != NULL && (strchr(fromname, '#') == NULL && strchr(fromname, '@') == NULL)) { /* @@ -4454,6 +4436,10 @@ zfs_do_send(int argc, char **argv) (void) strlcat(frombuf, tmpbuf, sizeof (frombuf)); fromname = frombuf; } + + zhp = zfs_open(g_zfs, argv[0], ZFS_TYPE_DATASET); + if (zhp == NULL) + return (1); err = zfs_send_one(zhp, fromname, STDOUT_FILENO, &flags, redactbook); zfs_close(zhp); diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index 4571541716..ce6a2737b8 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -2816,6 +2816,7 @@ zfs_send_one(zfs_handle_t *zhp, const char *from, int fd, sendflags_t *flags, { int err; libzfs_handle_t *hdl = zhp->zfs_hdl; + char *name = zhp->zfs_name; int orig_fd = fd; pthread_t ddtid, ptid; progress_arg_t pa = { 0 }; @@ -2823,7 +2824,7 @@ zfs_send_one(zfs_handle_t *zhp, const char *from, int fd, sendflags_t *flags, char errbuf[1024]; (void) snprintf(errbuf, sizeof (errbuf), dgettext(TEXT_DOMAIN, - "warning: cannot send '%s'"), zhp->zfs_name); + "warning: cannot send '%s'"), name); if (from != NULL && strchr(from, '@')) { zfs_handle_t *from_zhp = zfs_open(hdl, from, @@ -2839,6 +2840,44 @@ zfs_send_one(zfs_handle_t *zhp, const char *from, int fd, sendflags_t *flags, zfs_close(from_zhp); } + if (redactbook != NULL) { + char bookname[ZFS_MAX_DATASET_NAME_LEN]; + nvlist_t *redact_snaps; + zfs_handle_t *book_zhp; + char *at, *pound; + int dsnamelen; + + pound = strchr(redactbook, '#'); + if (pound != NULL) + redactbook = pound + 1; + at = strchr(name, '@'); + if (at == NULL) { + zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, + "cannot do a redacted send to a filesystem")); + return (zfs_error(hdl, EZFS_BADTYPE, errbuf)); + } + dsnamelen = at - name; + if (snprintf(bookname, sizeof (bookname), "%.*s#%s", + dsnamelen, name, redactbook) + >= sizeof (bookname)) { + zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, + "invalid bookmark name")); + return (zfs_error(hdl, EZFS_INVALIDNAME, errbuf)); + } + book_zhp = zfs_open(hdl, 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); + zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, + "not a redaction bookmark")); + return (zfs_error(hdl, EZFS_BADTYPE, errbuf)); + } + zfs_close(book_zhp); + } + /* * Send fs properties */ @@ -2909,7 +2948,7 @@ zfs_send_one(zfs_handle_t *zhp, const char *from, int fd, sendflags_t *flags, } } - err = lzc_send_redacted(zhp->zfs_name, from, fd, + err = lzc_send_redacted(name, from, fd, lzc_flags_from_sendflags(flags), redactbook); if (flags->progress) { @@ -2948,7 +2987,7 @@ zfs_send_one(zfs_handle_t *zhp, const char *from, int fd, sendflags_t *flags, case ENOENT: case ESRCH: - if (lzc_exists(zhp->zfs_name)) { + if (lzc_exists(name)) { zfs_error_aux(hdl, dgettext(TEXT_DOMAIN, "incremental source (%s) does not exist"), from); @@ -2967,7 +3006,9 @@ zfs_send_one(zfs_handle_t *zhp, const char *from, int fd, sendflags_t *flags, return (zfs_error(hdl, EZFS_BUSY, errbuf)); case EDQUOT: + case EFAULT: case EFBIG: + case EINVAL: case EIO: case ENOLINK: case ENOSPC: @@ -2975,7 +3016,6 @@ zfs_send_one(zfs_handle_t *zhp, const char *from, int fd, sendflags_t *flags, case ENXIO: case EPIPE: case ERANGE: - case EFAULT: case EROFS: zfs_error_aux(hdl, strerror(errno)); return (zfs_error(hdl, EZFS_BADBACKUP, errbuf)); 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 f3d516e958..10e93337ab 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 @@ -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 index 5ea65e3e72..56b990be1b 100755 --- 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