zstream: report invalid record context without assertions

Stream framing and BEGIN payload size depend on the input.  Report
invalid values as ordinary errors with the record offset instead of
terminating through VERIFY assertions.

Reject oversized BEGIN payloads before allocating or reading them.
Track compound-stream conclusions explicitly so valid compound sends
remain accepted while stray END records and records after a conclusion
are rejected.

Add ZTS coverage for malformed streams in both byte orders and for
valid compound streams.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Garth Snyder <[email protected]>
Signed-off-by: Matthias Goergens <[email protected]>
Closes #19097

cmd/zstream/zstream_io.c

@@ -45,6 +45,7 @@
 #define	MEMORY_BASE_CUTOFF	(4ULL << 30)	/* 4GB */
 #define	MEMORY_PCT		10		/* % beyond the base region */
 #define	MEMORY_HYSTERESIS 	(128 << 20)	/* 128MB */
+#define	BEGIN_PAYLOAD_MAX	(1UL << 28)
 
 /*
  * Init only the filename; chain functions will prepare the FILE *
@@ -372,8 +373,13 @@ chain_read(void *item_in, void *context_in)
 	if (context->ic_offset == 0)
 		set_stream_attributes(item);
 
+	uint32_t drr_type = ATTR_IS_SET(CA_BYTESWAPPED) ?
+	    BSWAP_32(drr->drr_type) : drr->drr_type;
 	size_t payload_size = calc_payload_size(drr);
-	if (payload_size > UINT32_MAX) {
+	if (drr_type == DRR_BEGIN && payload_size > BEGIN_PAYLOAD_MAX) {
+		errx(1, "DRR_BEGIN payload too large at offset %llu",
+		    (u_longlong_t)context->ic_offset);
+	} else if (payload_size > UINT32_MAX) {
 		errx(1, "stated packet size is greater than uint32_t "
 		    "at offset %llu", (u_longlong_t)context->ic_offset);
 	} else if (payload_size > 0) {
@@ -383,8 +389,6 @@ chain_read(void *item_in, void *context_in)
 		set_payload(item, buff, payload_size);
 	}
 
-	uint32_t drr_type = ATTR_IS_SET(CA_BYTESWAPPED) ?
-	    BSWAP_32(drr->drr_type) : drr->drr_type;
 	if (drr_type >= DRR_NUMTYPES) {
 		err(1, "invalid record type %llu found at offset %llu",
 		    (u_longlong_t)drr_type, (u_longlong_t)context->ic_offset);

cmd/zstream/zstream_validate.c

@@ -34,6 +34,8 @@ typedef struct {
 	int	nesting;
 	uint64_t featureflags;
 	boolean_t begin_spill;
+	boolean_t compound;
+	boolean_t concluded;
 } validate_context_t;
 
 static validate_context_t 	contexts[MAX_VALIDATIONS];
@@ -84,35 +86,55 @@ chain_validate_records(void *item_in, void *context_in)
 	if (OPTION_ENABLED(CA_DO_NOT_VALIDATE))
 		return (D_OK);
 
+	if (context->concluded) {
+		errx(1, "record after compound stream conclusion "
+		    "at offset %llu", (u_longlong_t)item->dp_stream_offset);
+	}
+
 	if (item->dp_stream_offset == 0 && drr->drr_type != DRR_BEGIN) {
 		warnx("warning - first record is not DRR_BEGIN");
 	}
 
 	if (drr->drr_type == DRR_BEGIN) {
-		VERIFY0(context->nesting);
+		if (context->nesting != 0) {
+			errx(1, "nested DRR_BEGIN record at offset %llu",
+			    (u_longlong_t)item->dp_stream_offset);
+		}
 		context->nesting++;
+		if (item->dp_stream_offset == 0) {
+			context->compound = DMU_GET_STREAM_HDRTYPE(
+			    drr->drr_u.drr_begin.drr_versioninfo) ==
+			    DMU_COMPOUNDSTREAM;
+		}
 		context->featureflags = DMU_GET_FEATUREFLAGS(
 		    drr->drr_u.drr_begin.drr_versioninfo);
 		context->begin_spill = !!(drr->drr_u.drr_begin.drr_flags &
 		    DRR_FLAG_SPILL_BLOCK);
 	} else if (drr->drr_type == DRR_END) {
 		VERIFY3S(context->nesting, >=, 0);
-		if (context->nesting > 0)
+		if (context->nesting > 0) {
 			context->nesting--;
+		} else if (context->compound &&
+		    IS_CONCLUSION(drr, DRR_END)) {
+			context->concluded = B_TRUE;
+		} else {
+			errx(1, "DRR_END record outside a stream "
+			    "at offset %llu",
+			    (u_longlong_t)item->dp_stream_offset);
+		}
 	} else if (drr->drr_type >= DRR_NUMTYPES) {
 		errx(1, "unknown record type: %d", drr->drr_type);
 	} else {
-		VERIFY3S(context->nesting, ==, 1);
+		if (context->nesting != 1) {
+			errx(1, "record outside a stream at offset %llu",
+			    (u_longlong_t)item->dp_stream_offset);
+		}
 	}
 
 	is_raw = validate_stream_has_feature(context, DMU_BACKUP_FEATURE_RAW);
 
 	switch (drr->drr_type) {
 
-	case DRR_BEGIN:
-		VERIFY3U(item->dp_payload_size, <=, 1UL << 28);
-		break;
-
 	case DRR_OBJECT:
 		err = recv_check_drr_object(drro, NULL, is_raw,
 		    context->begin_spill, context->featureflags, errbuf,
@@ -171,6 +193,8 @@ serial_validate_records(void)
 	context->nesting = 0;
 	context->featureflags = 0;
 	context->begin_spill = B_FALSE;
+	context->compound = B_FALSE;
+	context->concluded = B_FALSE;
 
 	chain_step_t step = {
 		.cs_type = CS_SERIAL,

tests/runfiles/common.run

@@ -1180,7 +1180,7 @@ tests = ['zstream_checksum_001_pos',
     'zstream_recompress_005_pos',
     'zstream_redup_001_pos',
     'zstream_selftest_queue_001_pos',
-    'zstream_validate_001_neg']
+    'zstream_validate_001_neg', 'zstream_validate_002_neg']
 tags = ['functional', 'zstream']
 
 [tests/functional/zvol/zvol_ENOSPC]

tests/zfs-tests/tests/Makefile.am

@@ -423,6 +423,7 @@ nobase_dist_datadir_zfs_tests_tests_DATA += \
 	functional/zstream/undersized-write-lsize.zsend.bz2 \
 	functional/zstream/big-endian-long-payloads.zsend.bz2 \
 	functional/zstream/little-endian-long-payloads.zsend.bz2 \
+	functional/zstream/make-invalid-context-streams.py \
 	functional/zstream/beadtbn-new.dump.bz2 \
 	functional/zstream/beadtbn-old.dump.bz2 \
 	functional/zstream/beadtbx-new.dump.bz2 \
@@ -2509,6 +2510,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
 	functional/zstream/zstream_recompress_005_pos.ksh \
 	functional/zstream/zstream_redup_001_pos.ksh \
 	functional/zstream/zstream_validate_001_neg.ksh \
+	functional/zstream/zstream_validate_002_neg.ksh \
 	functional/zstream/zstream_selftest_queue_001_pos.ksh \
 	functional/zvol/zvol_cli/cleanup.ksh \
 	functional/zvol/zvol_cli/setup.ksh \

tests/zfs-tests/tests/functional/zstream/make-invalid-context-streams.py

@@ -0,0 +1,74 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: CDDL-1.0
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# https://opensource.org/license/CDDL-1.0.
+#
+
+import struct
+import sys
+from pathlib import Path
+
+DRR_BEGIN = 0
+DRR_OBJECT = 1
+DRR_END = 5
+DMU_BACKUP_MAGIC = 0x2F5BACBAC
+DMU_SUBSTREAM_VERSIONINFO = 0x18C0011
+DMU_COMPOUNDSTREAM_VERSIONINFO = (
+    (DMU_SUBSTREAM_VERSIONINFO & ~0x3) | 0x2
+)
+BEGIN_PAYLOAD_MAX = 1 << 28
+RECORD_SIZE = 312
+
+
+def record(byte_order, record_type, payload_size=0):
+    header = struct.pack(f"{byte_order}II", record_type, payload_size)
+    return header + bytes(RECORD_SIZE - len(header))
+
+
+def begin(byte_order, payload_size=0, compound=False):
+    toname = b"test/source@baseline".ljust(256, b"\0")
+    value = struct.pack(
+        f"{byte_order}IIQQQIIQQ256s",
+        DRR_BEGIN,
+        payload_size,
+        DMU_BACKUP_MAGIC,
+        (DMU_COMPOUNDSTREAM_VERSIONINFO if compound
+         else DMU_SUBSTREAM_VERSIONINFO),
+        0,
+        2,
+        0,
+        1,
+        0,
+        toname,
+    )
+    assert len(value) == RECORD_SIZE
+    return value
+
+
+output = Path(sys.argv[1])
+output.mkdir(parents=True, exist_ok=True)
+for byte_order, name in ((">", "big"), ("<", "little")):
+    begin_record = begin(byte_order)
+    end_record = record(byte_order, DRR_END)
+    (output / f"nested-begin-{name}.zsend").write_bytes(
+        begin_record + begin_record
+    )
+    (output / f"record-after-end-{name}.zsend").write_bytes(
+        begin_record + end_record + record(byte_order, DRR_OBJECT)
+    )
+    (output / f"extra-end-{name}.zsend").write_bytes(
+        begin_record + end_record + end_record
+    )
+    (output / f"compound-extra-end-{name}.zsend").write_bytes(
+        begin(byte_order, compound=True) + end_record + end_record + end_record
+    )
+    (output / f"oversized-begin-{name}.zsend").write_bytes(
+        begin(byte_order, BEGIN_PAYLOAD_MAX + 1)
+    )

tests/zfs-tests/tests/functional/zstream/zstream_validate_002_neg.ksh

@@ -0,0 +1,69 @@
+#!/bin/ksh -p
+# SPDX-License-Identifier: CDDL-1.0
+
+#
+# This file and its contents are supplied under the terms of the
+# Common Development and Distribution License ("CDDL"), version 1.0.
+# You may only use this file in accordance with the terms of version
+# 1.0 of the CDDL.
+#
+# A full copy of the text of the CDDL should have accompanied this
+# source.  A copy of the CDDL is also available via the Internet at
+# https://opensource.org/license/CDDL-1.0.
+#
+
+. $STF_SUITE/tests/functional/zstream/zstream.kshlib
+
+#
+# Description:
+# Verify that zstream dump rejects invalid record context and oversized BEGIN
+# payloads while accepting valid compound streams.
+#
+# Strategy:
+# 1. Generate malformed stream records in both byte orders
+# 2. Check context and payload-size errors include the record offset
+# 3. Check existing valid compound-stream fixtures remain accepted
+#
+
+verify_runnable "both"
+
+log_assert "zstream rejects invalid record context and oversized payloads"
+
+typeset streams=$BACKDIR/context-streams
+typeset err=$BACKDIR/context-error
+typeset generator=$ZSTREAM_DATADIR/make-invalid-context-streams.py
+
+function verify_rejected # stream message
+{
+	typeset stream=$1
+	typeset message=$2
+	typeset ret
+
+	zstream dump -C "$stream" >/dev/null 2>"$err"
+	ret=$?
+	log_must test "$ret" -eq 1
+	log_must grep -q "$message" "$err"
+}
+
+log_must python3 "$generator" "$streams"
+
+for endian in big little; do
+	verify_rejected "$streams/nested-begin-$endian.zsend" \
+	    "nested DRR_BEGIN record at offset 312"
+	verify_rejected "$streams/record-after-end-$endian.zsend" \
+	    "record outside a stream at offset 624"
+	verify_rejected "$streams/extra-end-$endian.zsend" \
+	    "DRR_END record outside a stream at offset 624"
+	verify_rejected "$streams/compound-extra-end-$endian.zsend" \
+	    "record after compound stream conclusion at offset 936"
+	verify_rejected "$streams/oversized-begin-$endian.zsend" \
+	    "DRR_BEGIN payload too large at offset 0"
+done
+
+for endian in big little; do
+	typeset stream="${endian}-endian-all-drr-types-incr-XDR.zsend.bz2"
+	typeset compound="$ZSTREAM_DATADIR/$stream"
+	log_must eval "bzcat $compound | zstream dump >/dev/null"
+done
+
+log_pass "zstream reports invalid stream context with record offsets"