firmware: VB2_REC_OR_DIE() should not abort before vb2_check_recovery()

Unfortunately, CL:3168437 introduced a new problem when booting with a
broken TPM: secdata accessors no longer return failure but instead just
abort when booting in normal mode and continue when we're in recovery
mode. The problem is that when accessing secdata very early in
vb2api_fw_phase1(), we have not decided whether we're booting in
recovery mode yet. If vb2_secdata_firmware_init() fails, we will call
vb2api_fail() and then continue knowing that vb2_check_recovery() will
later see the recovery reason in NVRAM and decide to boot directly into
recovery from here. But if the code in-between accesses secdata, the
VB2_CONTEXT_RECOVERY_MODE flag is technically not yet set, so our
secdata accessor thinks we are booting in normal mode and something
terrible happened (because it shouldn't be possible to boot in normal
mode when secdata_init failed), so it aborts.

In order to try to solve this problem in a more general way, introduce a
new VB2_SD_STATUS_RECOVERY_DECIDED status flag that gets set once we
reach the point where we have conclusively decided whether we are
booting into recovery mode and set the appropriate context flags. Any
code using VB2_REC_OR_DIE() before that point will play it safe and
assume that we may still go into recovery mode, so we shouldn't abort.

BRANCH=none
BUG=none
TEST=none

Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: Ic3daa8dac932286257cbceebfff8712d25c3a97a
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/3301540
Reviewed-by: Hsuan Ting Chen <roccochen@chromium.org>
Reviewed-by: Yu-Ping Wu <yupingso@chromium.org>

9 files changed, 124 insertions, 5 deletions

diff --git a/firmware/2lib/2misc.c b/firmware/2lib/2misc.c
index 265db317..4896694b 100644
--- a/firmware/2lib/2misc.c
+++ b/firmware/2lib/2misc.c
@@ -164,6 +164,8 @@ void vb2_check_recovery(struct vb2_context *ctx)
 			  sd->recovery_reason,
 			  vb2_nv_get(ctx, VB2_NV_RECOVERY_SUBCODE));
 	}
+
+	sd->status |= VB2_SD_STATUS_RECOVERY_DECIDED;
 }
 
 vb2_error_t vb2_fw_init_gbb(struct vb2_context *ctx)
diff --git a/firmware/2lib/include/2common.h b/firmware/2lib/include/2common.h
index 695f50d1..d43f18a5 100644
--- a/firmware/2lib/include/2common.h
+++ b/firmware/2lib/include/2common.h
@@ -10,6 +10,7 @@
 
 #include "2api.h"
 #include "2gbb.h"
+#include "2misc.h"
 #include "2packed_key.h"
 #include "2return_codes.h"
 #include "2sha.h"
@@ -66,7 +67,8 @@ struct vb2_public_key;
 
 #define VB2_REC_OR_DIE(ctx, format, args...) do { \
 	VB2_DEBUG(format, ## args); \
-	if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) { \
+	if ((vb2_get_sd(ctx)->status & VB2_SD_STATUS_RECOVERY_DECIDED) && \
+	    !(ctx->flags & VB2_CONTEXT_RECOVERY_MODE)) { \
 		vb2ex_abort(); \
 		for (;;); \
 	} \
diff --git a/firmware/2lib/include/2struct.h b/firmware/2lib/include/2struct.h
index ea193d74..08a1efe3 100644
--- a/firmware/2lib/include/2struct.h
+++ b/firmware/2lib/include/2struct.h
@@ -85,6 +85,8 @@ enum vb2_shared_data_status {
 	/* EC Sync completed successfully */
 	VB2_SD_STATUS_EC_SYNC_COMPLETE = (1 << 6),
 
+	/* Have checked whether we are booting into recovery mode or not. */
+	VB2_SD_STATUS_RECOVERY_DECIDED = (1 << 7),
 };
 
 /* "V2SD" = vb2_shared_data.magic */
diff --git a/tests/vb20_api_kernel_tests.c b/tests/vb20_api_kernel_tests.c
index 893cd4e4..b835251f 100644
--- a/tests/vb20_api_kernel_tests.c
+++ b/tests/vb20_api_kernel_tests.c
@@ -59,6 +59,7 @@ static void reset_common_data(enum reset_type t)
 		  "vb2api_init failed");
 
 	sd = vb2_get_sd(ctx);
+	sd->status |= VB2_SD_STATUS_RECOVERY_DECIDED;
 	vb2_nv_init(ctx);
 
 	vb2api_secdata_kernel_create(ctx);
diff --git a/tests/vb2_api_tests.c b/tests/vb2_api_tests.c
index beab239d..15c9bd1f 100644
--- a/tests/vb2_api_tests.c
+++ b/tests/vb2_api_tests.c
@@ -374,6 +374,8 @@ static void phase1_tests(void)
 		 0, "  secdata firmware initialized");
 	TEST_NEQ(sd->status & VB2_SD_STATUS_SECDATA_KERNEL_INIT,
 		 0, "  secdata kernel initialized");
+	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		 0, "  recovery decided");
 
 	reset_common_data(FOR_MISC);
 	retval_vb2_fw_init_gbb = VB2_ERROR_GBB_MAGIC;
@@ -383,6 +385,8 @@ static void phase1_tests(void)
 		"  recovery reason");
 	TEST_NEQ(ctx->flags & VB2_CONTEXT_RECOVERY_MODE, 0, "  recovery flag");
 	TEST_NEQ(ctx->flags & VB2_CONTEXT_CLEAR_RAM, 0, "  clear ram flag");
+	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		 0, "  recovery decided");
 
 	/* Dev switch error proceeds to a recovery boot */
 	reset_common_data(FOR_MISC);
@@ -398,6 +402,8 @@ static void phase1_tests(void)
 		 0, "  display init context flag");
 	TEST_NEQ(sd->flags & VB2_SD_FLAG_DISPLAY_AVAILABLE,
 		 0, "  display available SD flag");
+	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		 0, "  recovery decided");
 
 	reset_common_data(FOR_MISC);
 	ctx->secdata_firmware[0] ^= 0x42;
@@ -407,6 +413,8 @@ static void phase1_tests(void)
 		"  recovery reason");
 	TEST_NEQ(ctx->flags & VB2_CONTEXT_RECOVERY_MODE, 0, "  recovery flag");
 	TEST_NEQ(ctx->flags & VB2_CONTEXT_CLEAR_RAM, 0, "  clear ram flag");
+	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		 0, "  recovery decided");
 
 	/* Bad secdata_kernel causes recovery mode */
 	reset_common_data(FOR_MISC);
@@ -417,6 +425,8 @@ static void phase1_tests(void)
 		"  recovery reason");
 	TEST_NEQ(ctx->flags & VB2_CONTEXT_RECOVERY_MODE, 0, "  recovery flag");
 	TEST_NEQ(ctx->flags & VB2_CONTEXT_CLEAR_RAM, 0, "  clear ram flag");
+	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		 0, "  recovery decided");
 
 	/* Test secdata_firmware-requested reboot */
 	reset_common_data(FOR_MISC);
@@ -428,6 +438,8 @@ static void phase1_tests(void)
 		1, "  tpm reboot request");
 	TEST_EQ(vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST),
 		0, "  recovery request");
+	TEST_EQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		0, "  recovery decided");
 
 	reset_common_data(FOR_MISC);
 	vb2_nv_set(ctx, VB2_NV_TPM_REQUESTED_REBOOT, 1);
@@ -438,6 +450,8 @@ static void phase1_tests(void)
 		0, "  tpm reboot request");
 	TEST_EQ(vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST),
 		0, "  recovery request");
+	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		 0, "  recovery decided");
 
 	reset_common_data(FOR_MISC);
 	ctx->flags |= VB2_CONTEXT_SECDATA_WANTS_REBOOT;
@@ -450,6 +464,8 @@ static void phase1_tests(void)
 		1, "  tpm reboot request");
 	TEST_EQ(vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST),
 		0, "  recovery request");
+	TEST_EQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		0, "  recovery decided");
 
 	reset_common_data(FOR_MISC);
 	ctx->flags |= VB2_CONTEXT_SECDATA_WANTS_REBOOT;
@@ -462,6 +478,8 @@ static void phase1_tests(void)
 		1, "  tpm reboot request");
 	TEST_EQ(vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST),
 		VB2_RECOVERY_RO_TPM_REBOOT, "  recovery request");
+	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		 0, "  recovery decided");
 
 	reset_common_data(FOR_MISC);
 	ctx->flags |= VB2_CONTEXT_SECDATA_WANTS_REBOOT;
@@ -474,6 +492,8 @@ static void phase1_tests(void)
 		1, "  tpm reboot request");
 	TEST_EQ(vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST),
 		VB2_RECOVERY_RO_UNSPECIFIED, "  recovery request");
+	TEST_EQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		0, "  recovery decided");
 
 	reset_common_data(FOR_MISC);
 	vb2_nv_set(ctx, VB2_NV_TPM_REQUESTED_REBOOT, 1);
@@ -486,6 +506,8 @@ static void phase1_tests(void)
 		0, "  tpm reboot request");
 	TEST_EQ(vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST),
 		VB2_RECOVERY_RO_UNSPECIFIED, "  recovery request");
+	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		 0, "  recovery decided");
 
 	reset_common_data(FOR_MISC);
 	ctx->flags |= VB2_CONTEXT_SECDATA_WANTS_REBOOT;
@@ -499,6 +521,8 @@ static void phase1_tests(void)
 		1, "  tpm reboot request");
 	TEST_EQ(vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST),
 		VB2_RECOVERY_RO_UNSPECIFIED, "  recovery request");
+	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		 0, "  recovery decided");
 
 	/* Cases for checking DISPLAY_INIT and DISPLAY_AVAILABLE. */
 	reset_common_data(FOR_MISC);
@@ -508,6 +532,8 @@ static void phase1_tests(void)
 		 0, "  display init context flag");
 	TEST_NEQ(sd->flags & VB2_SD_FLAG_DISPLAY_AVAILABLE,
 		 0, "  display available SD flag");
+	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		 0, "  recovery decided");
 
 	reset_common_data(FOR_MISC);
 	vb2_nv_set(ctx, VB2_NV_DISPLAY_REQUEST, 1);
@@ -516,6 +542,8 @@ static void phase1_tests(void)
 		 0, "  display init context flag");
 	TEST_NEQ(sd->flags & VB2_SD_FLAG_DISPLAY_AVAILABLE,
 		 0, "  display available SD flag");
+	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		 0, "  recovery decided");
 
 	reset_common_data(FOR_MISC);
 	force_dev_mode = 1;
@@ -524,6 +552,8 @@ static void phase1_tests(void)
 		 0, "  display init context flag");
 	TEST_NEQ(sd->flags & VB2_SD_FLAG_DISPLAY_AVAILABLE,
 		 0, "  display available SD flag");
+	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		 0, "  recovery decided");
 }
 
 static void phase2_tests(void)
diff --git a/tests/vb2_misc_tests.c b/tests/vb2_misc_tests.c
index d7afeb68..c552301e 100644
--- a/tests/vb2_misc_tests.c
+++ b/tests/vb2_misc_tests.c
@@ -40,7 +40,7 @@ static void reset_common_data(void)
 		  "vb2api_init failed");
 
 	sd = vb2_get_sd(ctx);
-	sd->status = VB2_SD_STATUS_SECDATA_FWMP_INIT;
+	sd->status |= VB2_SD_STATUS_SECDATA_FWMP_INIT;
 
 	memset(&gbb, 0, sizeof(gbb));
 
@@ -268,12 +268,17 @@ static void misc_tests(void)
 		"vb_workbuf_from_ctx() size");
 
 	reset_common_data();
+	sd->status |= VB2_SD_STATUS_RECOVERY_DECIDED;
 	TEST_ABORT(VB2_REC_OR_DIE(ctx, "die\n"), "REC_OR_DIE in normal mode");
 
 	reset_common_data();
+	sd->status |= VB2_SD_STATUS_RECOVERY_DECIDED;
 	ctx->flags |= VB2_CONTEXT_RECOVERY_MODE;
 	VB2_REC_OR_DIE(ctx, "VB2_REC_OR_DIE() test in recovery mode\n");
 	/* Would exit here if it didn't work as intended. */
+
+	reset_common_data();
+	VB2_REC_OR_DIE(ctx, "VB2_REC_OR_DIE() test in fw_phase1\n");
 }
 
 static void gbb_tests(void)
@@ -421,12 +426,16 @@ static void recovery_tests(void)
 {
 	/* No recovery */
 	reset_common_data();
+	TEST_EQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		0, "recovery not yet decided before testing check_recovery()");
 	vb2_check_recovery(ctx);
 	TEST_EQ(sd->recovery_reason, 0, "No recovery reason");
 	TEST_EQ(sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY,
 		0, "Not manual recovery");
 	TEST_EQ(ctx->flags & VB2_CONTEXT_RECOVERY_MODE,
 		0, "Not recovery mode");
+	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		 0, "Recovery decided");
 
 	/* From request */
 	reset_common_data();
@@ -438,6 +447,8 @@ static void recovery_tests(void)
 		0, "Not manual recovery");
 	TEST_NEQ(ctx->flags & VB2_CONTEXT_RECOVERY_MODE,
 		 0, "Recovery mode");
+	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		 0, "Recovery decided");
 
 	/* From request, but already failed */
 	reset_common_data();
@@ -447,6 +458,8 @@ static void recovery_tests(void)
 	TEST_EQ(sd->recovery_reason, 5, "Recovery reason already failed");
 	TEST_EQ(vb2_nv_get(ctx, VB2_NV_RECOVERY_REQUEST),
 		4, "NV not cleared");
+	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		 0, "Recovery decided");
 
 	/* Override */
 	reset_common_data();
@@ -457,6 +470,8 @@ static void recovery_tests(void)
 		"Recovery reason forced");
 	TEST_NEQ(sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY,
 		 0, "SD flag set");
+	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		 0, "Recovery decided");
 
 	/* Override subcode TRAIN_AND_REBOOT */
 	reset_common_data();
@@ -467,6 +482,8 @@ static void recovery_tests(void)
 		"Recovery reason forced");
 	TEST_NEQ(sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY,
 		 0, "SD flag set");
+	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		 0, "Recovery decided");
 
 	/* Promote subcode from BROKEN screen*/
 	reset_common_data();
@@ -477,6 +494,8 @@ static void recovery_tests(void)
 		"Recovery reason forced from BROKEN");
 	TEST_NEQ(sd->flags & VB2_SD_FLAG_MANUAL_RECOVERY,
 		 0, "SD flag set");
+	TEST_NEQ(sd->status & VB2_SD_STATUS_RECOVERY_DECIDED,
+		 0, "Recovery decided");
 }
 
 static void dev_switch_tests(void)
@@ -589,13 +608,15 @@ static void dev_switch_tests(void)
 
 	/*
 	 * secdata_firmware failure in normal mode fails and shows dev=0 even
-	 * if dev mode was on in the (inaccessible) secdata_firmware.
+	 * if dev mode was on in the (inaccessible) secdata_firmware. Since this
+	 * happens in fw_phase1, we do not abort -- we know that when secdata
+	 * is uninitialized here, we must be headed for recovery mode.
 	 */
 	reset_common_data();
 	vb2_secdata_firmware_set(ctx, VB2_SECDATA_FIRMWARE_FLAGS,
 				 VB2_SECDATA_FIRMWARE_FLAG_DEV_MODE);
 	sd->status &= ~VB2_SD_STATUS_SECDATA_FIRMWARE_INIT;
-	TEST_ABORT(vb2_check_dev_switch(ctx), "secdata_firmware fail normal");
+	TEST_SUCC(vb2_check_dev_switch(ctx), "secdata_firmware fail normal");
 	TEST_EQ(sd->flags & VB2_SD_FLAG_DEV_MODE_ENABLED, 0, "  sd not in dev");
 	TEST_EQ(ctx->flags & VB2_CONTEXT_DEVELOPER_MODE, 0, "  ctx not in dev");
 
@@ -659,6 +680,7 @@ static void enable_dev_tests(void)
 	reset_common_data();
 	allow_recovery_retval = 1;
 	sd->status &= ~VB2_SD_STATUS_SECDATA_FIRMWARE_INIT;
+	sd->status |= VB2_SD_STATUS_RECOVERY_DECIDED;
 	TEST_ABORT(vb2api_enable_developer_mode(ctx),
 		   "secdata_firmware no init, enable dev mode aborted");
 	sd->status |= VB2_SD_STATUS_SECDATA_FIRMWARE_INIT;
diff --git a/tests/vb2_secdata_firmware_tests.c b/tests/vb2_secdata_firmware_tests.c
index 387cda36..c1b0a73f 100644
--- a/tests/vb2_secdata_firmware_tests.c
+++ b/tests/vb2_secdata_firmware_tests.c
@@ -28,6 +28,9 @@ static void reset_common_data(void)
 
 	sd = vb2_get_sd(ctx);
 
+	/* Most tests assume we have passed fw_phase1() */
+	sd->status |= VB2_SD_STATUS_RECOVERY_DECIDED;
+
 	sec = (struct vb2_secdata_firmware *)ctx->secdata_firmware;
 }
 
@@ -136,6 +139,25 @@ static void secdata_firmware_test(void)
 					    0x123456ff),
 		   "Set uninitialized");
 	test_changed(ctx, 0, "Set uninitialized doesn't change data");
+
+	/* Read/write uninitialized in recovery mode */
+	ctx->flags |= VB2_CONTEXT_RECOVERY_MODE;
+	TEST_EQ(vb2_secdata_firmware_get(ctx, VB2_SECDATA_FIRMWARE_VERSIONS), 0,
+		"Get uninitialized (recmode)");
+	test_changed(ctx, 0, "Get uninitialized (recmode) doesn't change data");
+	vb2_secdata_firmware_set(ctx, VB2_SECDATA_FIRMWARE_VERSIONS,
+				 0x123456ff);
+	test_changed(ctx, 0, "Set uninitialized (recmode) doesn't change data");
+
+	/* Read/write early in fw_phase1 */
+	ctx->flags &= ~VB2_CONTEXT_RECOVERY_MODE;
+	sd->status &= ~VB2_SD_STATUS_RECOVERY_DECIDED;
+	TEST_EQ(vb2_secdata_firmware_get(ctx, VB2_SECDATA_FIRMWARE_VERSIONS), 0,
+		"Get uninitialized (phase1)");
+	test_changed(ctx, 0, "Get uninitialized (phase1) doesn't change data");
+	vb2_secdata_firmware_set(ctx, VB2_SECDATA_FIRMWARE_VERSIONS,
+				 0x123456ff);
+	test_changed(ctx, 0, "Set uninitialized (phase1) doesn't change data");
 }
 
 int main(int argc, char* argv[])
diff --git a/tests/vb2_secdata_fwmp_tests.c b/tests/vb2_secdata_fwmp_tests.c
index 699f3fa7..79b0f55f 100644
--- a/tests/vb2_secdata_fwmp_tests.c
+++ b/tests/vb2_secdata_fwmp_tests.c
@@ -25,7 +25,8 @@ static void reset_common_data(void)
 		  "vb2api_init failed");
 
 	sd = vb2_get_sd(ctx);
-	sd->status = VB2_SD_STATUS_SECDATA_FWMP_INIT;
+	sd->status |= VB2_SD_STATUS_SECDATA_FWMP_INIT;
+	sd->status |= VB2_SD_STATUS_RECOVERY_DECIDED;
 
 	memset(&gbb, 0, sizeof(gbb));
 
@@ -199,6 +200,13 @@ static void get_flag_test(void)
 	sd->status &= ~VB2_SD_STATUS_SECDATA_FWMP_INIT;
 	TEST_ABORT(vb2_secdata_fwmp_get_flag(ctx, 0),
 		   "non-init in normal mode triggers abort");
+
+	/* FWMP hasn't been initialized (before recovery decision) */
+	reset_common_data();
+	sd->status &= ~VB2_SD_STATUS_SECDATA_FWMP_INIT;
+	sd->status &= ~VB2_SD_STATUS_RECOVERY_DECIDED;
+	TEST_EQ(vb2_secdata_fwmp_get_flag(ctx, 0), 0,
+		"non-init in fw_phase1 forces default flag value");
 }
 
 static void get_dev_key_hash_test(void)
@@ -222,6 +230,13 @@ static void get_dev_key_hash_test(void)
 	TEST_ABORT(vb2_secdata_fwmp_get_dev_key_hash(ctx),
 		   "non-init in normal mode triggers abort");
 
+	/* FWMP hasn't been initialized (before recovery decision) */
+	reset_common_data();
+	sd->status &= ~VB2_SD_STATUS_SECDATA_FWMP_INIT;
+	sd->status &= ~VB2_SD_STATUS_RECOVERY_DECIDED;
+	TEST_TRUE(vb2_secdata_fwmp_get_dev_key_hash(ctx) == NULL,
+		  "non-init in fw_phase1 forces NULL pointer");
+
 	/* Success case */
 	reset_common_data();
 	TEST_TRUE(vb2_secdata_fwmp_get_dev_key_hash(ctx) ==
diff --git a/tests/vb2_secdata_kernel_tests.c b/tests/vb2_secdata_kernel_tests.c
index 327018dc..dc41f1a1 100644
--- a/tests/vb2_secdata_kernel_tests.c
+++ b/tests/vb2_secdata_kernel_tests.c
@@ -30,6 +30,9 @@ static void reset_common_data(void)
 
 	sd = vb2_get_sd(ctx);
 
+	/* Most tests assume we have passed fw_phase1() */
+	sd->status |= VB2_SD_STATUS_RECOVERY_DECIDED;
+
 	sec02 = (struct vb2_secdata_kernel_v0 *)ctx->secdata_kernel;
 	sec10 = (struct vb2_secdata_kernel_v1 *)ctx->secdata_kernel;
 }
@@ -245,7 +248,27 @@ static void secdata_kernel_access_test_v10(void)
 		   "Set uninitialized");
 	test_changed(ctx, 0, "Set uninitialized doesn't change data");
 
+	/* Read/write uninitialized in recovery mode */
+	ctx->flags |= VB2_CONTEXT_RECOVERY_MODE;
+	TEST_EQ(vb2_secdata_kernel_get(ctx, VB2_SECDATA_KERNEL_VERSIONS), 0,
+		"Get uninitialized (recmode)");
+	test_changed(ctx, 0, "Get uninitialized (recmode) doesn't change data");
+	vb2_secdata_kernel_set(ctx, VB2_SECDATA_KERNEL_VERSIONS,
+				 0x123456ff);
+	test_changed(ctx, 0, "Set uninitialized (recmode) doesn't change data");
+
+	/* Read/write early in fw_phase1 */
+	ctx->flags &= ~VB2_CONTEXT_RECOVERY_MODE;
+	sd->status &= ~VB2_SD_STATUS_RECOVERY_DECIDED;
+	TEST_EQ(vb2_secdata_kernel_get(ctx, VB2_SECDATA_KERNEL_VERSIONS), 0,
+		"Get uninitialized (phase1)");
+	test_changed(ctx, 0, "Get uninitialized (phase1) doesn't change data");
+	vb2_secdata_kernel_set(ctx, VB2_SECDATA_KERNEL_VERSIONS,
+				 0x123456ff);
+	test_changed(ctx, 0, "Set uninitialized (phase1) doesn't change data");
+
 	/* Test EC hash set */
+	reset_common_data();
 	vb2api_secdata_kernel_create(ctx);
 	vb2_secdata_kernel_init(ctx);
 	memset(ec_hash, 0xaa, sizeof(ec_hash));