Skip to content

Commit

Permalink
Avoid core dump on invalid redaction bookmark
Browse files Browse the repository at this point in the history 
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 <pcd@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ryan Moeller <ryan@iXsystems.com>
Closes #10138
  • Loading branch information
Ryan Moeller authored Mar 18, 2020
1 parent 4df8b2c commit 22df245
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 25 deletions.
22 changes: 4 additions & 18 deletions cmd/zfs/zfs_main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
/*
Expand Down Expand Up @@ -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);
Expand Down
48 changes: 44 additions & 4 deletions lib/libzfs/libzfs_sendrecv.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2816,14 +2816,15 @@ 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 };
dedup_arg_t dda = { 0 };

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,
Expand All @@ -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
*/
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -2967,15 +3006,16 @@ 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:
case ENOSTR:
case ENXIO:
case EPIPE:
case ERANGE:
case EFAULT:
case EROFS:
zfs_error_aux(hdl, strerror(errno));
return (zfs_error(hdl, EZFS_BADBACKUP, errbuf));
Expand Down
4 changes: 2 additions & 2 deletions tests/zfs-tests/tests/functional/cli_root/zfs_bookmark/zfs_bookmark_cliargs.ksh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion tests/zfs-tests/tests/functional/redacted_send/redacted_negative.ksh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 22df245

Please sign in to comment.