Fix incremental receive silently failing for recursive sends

The problem occurs because dmu_recv_begin pulls in the payload and 
next header from the input stream in order to use the contents of 
the begin record's nvlist. However, the change to do that before the 
other checks in dmu_recv_begin occur caused a regression where an 
empty send stream in a recursive send could have its END record 
consumed by this, which broke the logic of recv_skip. A test is 
also included to protect against this case in the future.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Paul Dagnelie <pcd@delphix.com>
Closes #12661
Closes #14568

module/zfs/dmu_recv.c

@@ -1255,7 +1255,6 @@ dmu_recv_begin(char *tofs, char *tosnap, dmu_replay_record_t *drr_begin,
 	    DMU_GET_FEATUREFLAGS(drc->drc_drrb->drr_versioninfo);
 
 	uint32_t payloadlen = drc->drc_drr_begin->drr_payloadlen;
-	void *payload = NULL;
 
 	/*
 	 * Since OpenZFS 2.0.0, we have enforced a 64MB limit in userspace
@@ -1266,16 +1265,23 @@ dmu_recv_begin(char *tofs, char *tosnap, dmu_replay_record_t *drr_begin,
 	if (payloadlen > (MIN((1U << 28), arc_all_memory() / 4)))
 		return (E2BIG);
 
-	if (payloadlen != 0)
-		payload = vmem_alloc(payloadlen, KM_SLEEP);
 
-	err = receive_read_payload_and_next_header(drc, payloadlen,
-	    payload);
-	if (err != 0) {
-		vmem_free(payload, payloadlen);
-		return (err);
-	}
 	if (payloadlen != 0) {
+		void *payload = vmem_alloc(payloadlen, KM_SLEEP);
+		/*
+		 * For compatibility with recursive send streams, we don't do
+		 * this here if the stream could be part of a package. Instead,
+		 * we'll do it in dmu_recv_stream. If we pull the next header
+		 * too early, and it's the END record, we break the `recv_skip`
+		 * logic.
+		 */
+
+		err = receive_read_payload_and_next_header(drc, payloadlen,
+		    payload);
+		if (err != 0) {
+			vmem_free(payload, payloadlen);
+			return (err);
+		}
 		err = nvlist_unpack(payload, payloadlen, &drc->drc_begin_nvl,
 		    KM_SLEEP);
 		vmem_free(payload, payloadlen);
@@ -3315,6 +3321,17 @@ dmu_recv_stream(dmu_recv_cookie_t *drc, offset_t *voffp)
 			goto out;
 	}
 
+	/*
+	 * For compatibility with recursive send streams, we do this here,
+	 * rather than in dmu_recv_begin. If we pull the next header too
+	 * early, and it's the END record, we break the `recv_skip` logic.
+	 */
+	if (drc->drc_drr_begin->drr_payloadlen == 0) {
+		err = receive_read_payload_and_next_header(drc, 0, NULL);
+		if (err != 0)
+			goto out;
+	}
+
 	/*
 	 * If we failed before this point we will clean up any new resume
 	 * state that was created. Now that we've gotten past the initial

tests/runfiles/common.run

@@ -837,19 +837,20 @@ tests = ['recv_dedup', 'recv_dedup_encrypted_zvol', 'rsend_001_pos',
     'rsend_014_pos', 'rsend_016_neg', 'rsend_019_pos', 'rsend_020_pos',
     'rsend_021_pos', 'rsend_022_pos', 'rsend_024_pos', 'rsend_025_pos',
     'rsend_026_neg', 'rsend_027_pos', 'rsend_028_neg', 'rsend_029_neg',
-    'rsend_030_pos', 'send-c_verify_ratio', 'send-c_verify_contents',
+    'rsend_030_pos', 'rsend_031_pos', 'send-c_verify_ratio',
-    'send-c_props', 'send-c_incremental', 'send-c_volume',
+    'send-c_verify_contents', 'send-c_props', 'send-c_incremental',
-    'send-c_zstream_recompress', 'send-c_zstreamdump', 'send-c_lz4_disabled',
+    'send-c_volume', 'send-c_zstream_recompress', 'send-c_zstreamdump',
-    'send-c_recv_lz4_disabled', 'send-c_mixed_compression',
+    'send-c_lz4_disabled', 'send-c_recv_lz4_disabled',
-    'send-c_stream_size_estimate', 'send-c_embedded_blocks', 'send-c_resume',
+    'send-c_mixed_compression', 'send-c_stream_size_estimate',
-    'send-cpL_varied_recsize', 'send-c_recv_dedup', 'send-L_toggle',
+    'send-c_embedded_blocks', 'send-c_resume', 'send-cpL_varied_recsize',
-    'send_encrypted_incremental', 'send_encrypted_freeobjects',
+    'send-c_recv_dedup', 'send-L_toggle', 'send_encrypted_incremental',
-    'send_encrypted_hierarchy', 'send_encrypted_props',
+    'send_encrypted_freeobjects', 'send_encrypted_hierarchy',
-    'send_encrypted_truncated_files', 'send_freeobjects', 'send_realloc_files',
+    'send_encrypted_props', 'send_encrypted_truncated_files',
-    'send_realloc_encrypted_files', 'send_spill_block', 'send_holds',
+    'send_freeobjects', 'send_realloc_files', 'send_realloc_encrypted_files',
-    'send_hole_birth', 'send_mixed_raw', 'send-wR_encrypted_zvol',
+    'send_spill_block', 'send_holds', 'send_hole_birth', 'send_mixed_raw',
-    'send_partial_dataset', 'send_invalid', 'send_doall',
+    'send-wR_encrypted_zvol', 'send_partial_dataset', 'send_invalid',
-    'send_raw_spill_block', 'send_raw_ashift', 'send_raw_large_blocks']
+    'send_doall', 'send_raw_spill_block', 'send_raw_ashift',
+    'send_raw_large_blocks']
 tags = ['functional', 'rsend']
 
 [tests/functional/scrub_mirror]

tests/zfs-tests/cmd/libzfs_input_check.c

@@ -563,7 +563,7 @@ test_recv_new(const char *dataset, int fd)
 	fnvlist_add_uint64(optional, "action_handle", *action_handle);
 #endif
 	IOC_INPUT_TEST(ZFS_IOC_RECV_NEW, dataset, required, optional,
-	    ZFS_ERR_STREAM_TRUNCATED);
+	    ENOTSUP);
 
 	nvlist_free(props);
 	nvlist_free(optional);

tests/zfs-tests/tests/Makefile.am

@@ -1795,6 +1795,7 @@ nobase_dist_datadir_zfs_tests_tests_SCRIPTS += \
 	functional/rsend/rsend_028_neg.ksh \
 	functional/rsend/rsend_029_neg.ksh \
 	functional/rsend/rsend_030_pos.ksh \
+	functional/rsend/rsend_031_pos.ksh \
 	functional/rsend/send-c_embedded_blocks.ksh \
 	functional/rsend/send-c_incremental.ksh \
 	functional/rsend/send-c_lz4_disabled.ksh \

tests/zfs-tests/tests/functional/rsend/rsend_031_pos.ksh

@@ -0,0 +1,63 @@
+#!/bin/ksh -p
+
+#
+# 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
+# http://www.illumos.org/license/CDDL.
+#
+
+#
+# Copyright (c) 2023 by Delphix. All rights reserved.
+#
+
+. $STF_SUITE/tests/functional/rsend/rsend.kshlib
+
+#
+# Description:
+# Verify recursive incremental sends missing snapshots behave correctly.
+#
+# Strategy:
+# 1. Create snapshots on source filesystem.
+# 2. Recursively send snapshots.
+# 3. Delete snapshot on source filesystem
+# 4. Perform incremental recursive send.
+# 5. Verify matching snapshot lists.
+#
+
+verify_runnable "both"
+
+sendfs=$POOL/sendfs
+recvfs=$POOL2/recvfs
+
+function cleanup {
+	rm $BACKDIR/stream1
+	rm $BACKDIR/stream2
+	zfs destroy -r $sendfs
+	zfs destroy -r $recvfs
+}
+
+log_assert "Verify recursive incremental sends missing snapshots behave correctly."
+log_onexit cleanup
+
+log_must zfs create $sendfs
+log_must zfs snapshot $sendfs@A
+log_must zfs snapshot $sendfs@B
+log_must zfs snapshot $sendfs@C
+log_must eval "zfs send -Rpv $sendfs@C > $BACKDIR/stream1"
+log_must eval "zfs receive -F $recvfs < $BACKDIR/stream1"
+log_must zfs list $sendfs@C
+
+log_must zfs destroy $sendfs@C
+log_must zfs snapshot $sendfs@D
+log_must zfs snapshot $sendfs@E
+log_must eval "zfs send -Rpv -I $sendfs@A $sendfs@E > $BACKDIR/stream2"
+log_must eval "zfs receive -Fv $recvfs < $BACKDIR/stream2"
+log_must zfs list $sendfs@D
+log_must zfs list $sendfs@E
+log_mustnot zfs list $sendfs@C
+log_pass "Verify recursive incremental sends missing snapshots behave correctly."