Address theoretical uninitialized variable usage in zstream

Coverity has long complained about the checksum being uninitialized if
an END record is processed before its BEGIN record. This should not
happen, but there was no code to check for it. I had left this unfixed
since it was a low priority issue, but then
9f4ede63d23be4f43ba8dd0ca42c6a773a8eaa8d added another instance of this.

I am making an effort to "hold the line" to keep new coverity defect
reports from going unaddressed, so I find myself forced to fix this much
earlier than I had originally planned to address it.

The solution is to maintain a counter and a flag. Then use VERIFY
statements to verify the following runtime constraints:

 * Every record either has a corresponding BEGIN record, is a BEGIN
   record or is the end of stream END record for replication streams.
 * BEGIN records cannot be nested. i.e. There must be an END record
   before another BEGIN record may be seen.

Failure to meet these constraints will cause the program to exit.

This is sufficient to ensure that the checksum is never accessed when
uninitialized.

Reported-by: Coverity (CID 1524578)
Reported-by: Coverity (CID 1524633)
Reported-by: Coverity (CID 1527295)
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Damian Szuberski <[email protected]>
Signed-off-by: Richard Yao <[email protected]>
Closes #14176 

3 files changed, 51 insertions, 0 deletions

diff --git a/cmd/zstream/zstream_decompress.c b/cmd/zstream/zstream_decompress.c
index 6e0da0852..726c3be6d 100644
--- a/cmd/zstream/zstream_decompress.c
+++ b/cmd/zstream/zstream_decompress.c
@@ -158,6 +158,8 @@ zstream_do_decompress(int argc, char *argv[])
 	}
 
 	fletcher_4_init();
+	int begin = 0;
+	boolean_t seen = B_FALSE;
 	while (sfread(drr, sizeof (*drr), stdin) != 0) {
 		struct drr_write *drrw;
 		uint64_t payload_size = 0;
@@ -174,6 +176,8 @@ zstream_do_decompress(int argc, char *argv[])
 		case DRR_BEGIN:
 		{
 			ZIO_SET_CHECKSUM(&stream_cksum, 0, 0, 0, 0);
+			VERIFY0(begin++);
+			seen = B_TRUE;
 
 			int sz = drr->drr_payloadlen;
 			if (sz != 0) {
@@ -192,6 +196,13 @@ zstream_do_decompress(int argc, char *argv[])
 		{
 			struct drr_end *drre = &drr->drr_u.drr_end;
 			/*
+			 * We would prefer to just check --begin == 0, but
+			 * replication streams have an end of stream END
+			 * record, so we must avoid tripping it.
+			 */
+			VERIFY3B(seen, ==, B_TRUE);
+			begin--;
+			/*
 			 * Use the recalculated checksum, unless this is
 			 * the END record of a stream package, which has
 			 * no checksum.
@@ -204,6 +215,7 @@ zstream_do_decompress(int argc, char *argv[])
 		case DRR_OBJECT:
 		{
 			struct drr_object *drro = &drr->drr_u.drr_object;
+			VERIFY3S(begin, ==, 1);
 
 			if (drro->drr_bonuslen > 0) {
 				payload_size = DRR_OBJECT_PAYLOAD_SIZE(drro);
@@ -215,12 +227,14 @@ zstream_do_decompress(int argc, char *argv[])
 		case DRR_SPILL:
 		{
 			struct drr_spill *drrs = &drr->drr_u.drr_spill;
+			VERIFY3S(begin, ==, 1);
 			payload_size = DRR_SPILL_PAYLOAD_SIZE(drrs);
 			(void) sfread(buf, payload_size, stdin);
 			break;
 		}
 
 		case DRR_WRITE_BYREF:
+			VERIFY3S(begin, ==, 1);
 			fprintf(stderr,
 			    "Deduplicated streams are not supported\n");
 			exit(1);
@@ -228,6 +242,7 @@ zstream_do_decompress(int argc, char *argv[])
 
 		case DRR_WRITE:
 		{
+			VERIFY3S(begin, ==, 1);
 			drrw = &thedrr.drr_u.drr_write;
 			payload_size = DRR_WRITE_PAYLOAD_SIZE(drrw);
 			ENTRY *p;
@@ -321,6 +336,7 @@ zstream_do_decompress(int argc, char *argv[])
 
 		case DRR_WRITE_EMBEDDED:
 		{
+			VERIFY3S(begin, ==, 1);
 			struct drr_write_embedded *drrwe =
 			    &drr->drr_u.drr_write_embedded;
 			payload_size =
@@ -332,6 +348,7 @@ zstream_do_decompress(int argc, char *argv[])
 		case DRR_FREEOBJECTS:
 		case DRR_FREE:
 		case DRR_OBJECT_RANGE:
+			VERIFY3S(begin, ==, 1);
 			break;
 
 		default:
diff --git a/cmd/zstream/zstream_recompress.c b/cmd/zstream/zstream_recompress.c
index b7370587f..38ef758f8 100644
--- a/cmd/zstream/zstream_recompress.c
+++ b/cmd/zstream/zstream_recompress.c
@@ -138,6 +138,8 @@ zstream_do_recompress(int argc, char *argv[])
 	fletcher_4_init();
 	zio_init();
 	zstd_init();
+	int begin = 0;
+	boolean_t seen = B_FALSE;
 	while (sfread(drr, sizeof (*drr), stdin) != 0) {
 		struct drr_write *drrw;
 		uint64_t payload_size = 0;
@@ -155,6 +157,8 @@ zstream_do_recompress(int argc, char *argv[])
 		case DRR_BEGIN:
 		{
 			ZIO_SET_CHECKSUM(&stream_cksum, 0, 0, 0, 0);
+			VERIFY0(begin++);
+			seen = B_TRUE;
 
 			int sz = drr->drr_payloadlen;
 			if (sz != 0) {
@@ -173,6 +177,13 @@ zstream_do_recompress(int argc, char *argv[])
 		{
 			struct drr_end *drre = &drr->drr_u.drr_end;
 			/*
+			 * We would prefer to just check --begin == 0, but
+			 * replication streams have an end of stream END
+			 * record, so we must avoid tripping it.
+			 */
+			VERIFY3B(seen, ==, B_TRUE);
+			begin--;
+			/*
 			 * Use the recalculated checksum, unless this is
 			 * the END record of a stream package, which has
 			 * no checksum.
@@ -185,6 +196,7 @@ zstream_do_recompress(int argc, char *argv[])
 		case DRR_OBJECT:
 		{
 			struct drr_object *drro = &drr->drr_u.drr_object;
+			VERIFY3S(begin, ==, 1);
 
 			if (drro->drr_bonuslen > 0) {
 				payload_size = DRR_OBJECT_PAYLOAD_SIZE(drro);
@@ -196,12 +208,14 @@ zstream_do_recompress(int argc, char *argv[])
 		case DRR_SPILL:
 		{
 			struct drr_spill *drrs = &drr->drr_u.drr_spill;
+			VERIFY3S(begin, ==, 1);
 			payload_size = DRR_SPILL_PAYLOAD_SIZE(drrs);
 			(void) sfread(buf, payload_size, stdin);
 			break;
 		}
 
 		case DRR_WRITE_BYREF:
+			VERIFY3S(begin, ==, 1);
 			fprintf(stderr,
 			    "Deduplicated streams are not supported\n");
 			exit(1);
@@ -209,6 +223,7 @@ zstream_do_recompress(int argc, char *argv[])
 
 		case DRR_WRITE:
 		{
+			VERIFY3S(begin, ==, 1);
 			drrw = &thedrr.drr_u.drr_write;
 			payload_size = DRR_WRITE_PAYLOAD_SIZE(drrw);
 			/*
@@ -295,6 +310,7 @@ zstream_do_recompress(int argc, char *argv[])
 		{
 			struct drr_write_embedded *drrwe =
 			    &drr->drr_u.drr_write_embedded;
+			VERIFY3S(begin, ==, 1);
 			payload_size =
 			    P2ROUNDUP((uint64_t)drrwe->drr_psize, 8);
 			(void) sfread(buf, payload_size, stdin);
@@ -304,6 +320,7 @@ zstream_do_recompress(int argc, char *argv[])
 		case DRR_FREEOBJECTS:
 		case DRR_FREE:
 		case DRR_OBJECT_RANGE:
+			VERIFY3S(begin, ==, 1);
 			break;
 
 		default:
diff --git a/cmd/zstream/zstream_redup.c b/cmd/zstream/zstream_redup.c
index 5807fabce..8b12303c5 100644
--- a/cmd/zstream/zstream_redup.c
+++ b/cmd/zstream/zstream_redup.c
@@ -222,6 +222,8 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
 	char *buf = safe_calloc(bufsz);
 	FILE *ofp = fdopen(infd, "r");
 	long offset = ftell(ofp);
+	int begin = 0;
+	boolean_t seen = B_FALSE;
 	while (sfread(drr, sizeof (*drr), ofp) != 0) {
 		num_records++;
 
@@ -240,6 +242,8 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
 			struct drr_begin *drrb = &drr->drr_u.drr_begin;
 			int fflags;
 			ZIO_SET_CHECKSUM(&stream_cksum, 0, 0, 0, 0);
+			VERIFY0(begin++);
+			seen = B_TRUE;
 
 			assert(drrb->drr_magic == DMU_BACKUP_MAGIC);
 
@@ -267,6 +271,13 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
 		{
 			struct drr_end *drre = &drr->drr_u.drr_end;
 			/*
+			 * We would prefer to just check --begin == 0, but
+			 * replication streams have an end of stream END
+			 * record, so we must avoid tripping it.
+			 */
+			VERIFY3B(seen, ==, B_TRUE);
+			begin--;
+			/*
 			 * Use the recalculated checksum, unless this is
 			 * the END record of a stream package, which has
 			 * no checksum.
@@ -279,6 +290,7 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
 		case DRR_OBJECT:
 		{
 			struct drr_object *drro = &drr->drr_u.drr_object;
+			VERIFY3S(begin, ==, 1);
 
 			if (drro->drr_bonuslen > 0) {
 				payload_size = DRR_OBJECT_PAYLOAD_SIZE(drro);
@@ -290,6 +302,7 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
 		case DRR_SPILL:
 		{
 			struct drr_spill *drrs = &drr->drr_u.drr_spill;
+			VERIFY3S(begin, ==, 1);
 			payload_size = DRR_SPILL_PAYLOAD_SIZE(drrs);
 			(void) sfread(buf, payload_size, ofp);
 			break;
@@ -299,6 +312,7 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
 		{
 			struct drr_write_byref drrwb =
 			    drr->drr_u.drr_write_byref;
+			VERIFY3S(begin, ==, 1);
 
 			num_write_byref_records++;
 
@@ -334,6 +348,7 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
 		case DRR_WRITE:
 		{
 			struct drr_write *drrw = &drr->drr_u.drr_write;
+			VERIFY3S(begin, ==, 1);
 			payload_size = DRR_WRITE_PAYLOAD_SIZE(drrw);
 			(void) sfread(buf, payload_size, ofp);
 
@@ -346,6 +361,7 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
 		{
 			struct drr_write_embedded *drrwe =
 			    &drr->drr_u.drr_write_embedded;
+			VERIFY3S(begin, ==, 1);
 			payload_size =
 			    P2ROUNDUP((uint64_t)drrwe->drr_psize, 8);
 			(void) sfread(buf, payload_size, ofp);
@@ -355,6 +371,7 @@ zfs_redup_stream(int infd, int outfd, boolean_t verbose)
 		case DRR_FREEOBJECTS:
 		case DRR_FREE:
 		case DRR_OBJECT_RANGE:
+			VERIFY3S(begin, ==, 1);
 			break;
 
 		default: