2misc: Abort before using GBB if gbb_offset is not initialized

In vb2_get_gbb, abort if gbb_offset is zero. This ensures that functions
like vb2api_gbb_get_flags won't try to read garbage GBB data if the
context hasn't been properly initialized.

Some additional changes made to fix tests:
1. In vb2_set_boot_mode, don't access GBB unless needed.
2. In vb2api_get_dev_default_boot_target, use vb2api_gbb_get_flags
   instead of vb2_get_gbb to make it easier to mock. This is needed for
   depthcharge tests.
3. Make vb2api_get_debug_info tolerant of GBB not being set. This is
   needed for depthcharge tests.

BUG=b:237093169
BRANCH=none
TEST=make && make runtests

Cq-Depend: chromium:3820402
Change-Id: I921d6cc4a5d91c8114c5e46748b4576a1e7716d0
Signed-off-by: Nicholas Bishop <nicholasbishop@google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/3817941
Reviewed-by: Julius Werner <jwerner@chromium.org>

3 files changed, 46 insertions, 10 deletions

diff --git a/Makefile b/Makefile
index df906a2f..95d14239 100644
--- a/Makefile
+++ b/Makefile
@@ -738,6 +738,7 @@ TEST2X_NAMES = \
 	tests/vb2_crypto_tests \
 	tests/vb2_ec_sync_tests \
 	tests/vb2_firmware_tests \
+	tests/vb2_gbb_init_tests \
 	tests/vb2_gbb_tests \
 	tests/vb2_host_flashrom_tests \
 	tests/vb2_host_key_tests \
@@ -1294,6 +1295,7 @@ run2tests: install_for_test
 	${RUNTEST} ${BUILD_RUN}/tests/vb2_crypto_tests
 	${RUNTEST} ${BUILD_RUN}/tests/vb2_ec_sync_tests
 	${RUNTEST} ${BUILD_RUN}/tests/vb2_firmware_tests
+	${RUNTEST} ${BUILD_RUN}/tests/vb2_gbb_init_tests
 	${RUNTEST} ${BUILD_RUN}/tests/vb2_gbb_tests
 	${RUNTEST} ${BUILD_RUN}/tests/vb2_host_key_tests
 	${RUNTEST} ${BUILD_RUN}/tests/vb2_load_kernel_tests
diff --git a/firmware/2lib/2misc.c b/firmware/2lib/2misc.c
index 57351278..62a1ab3e 100644
--- a/firmware/2lib/2misc.c
+++ b/firmware/2lib/2misc.c
@@ -34,6 +34,8 @@ test_mockable
 struct vb2_gbb_header *vb2_get_gbb(struct vb2_context *ctx)
 {
 	struct vb2_shared_data *sd = vb2_get_sd(ctx);
+	if (sd->gbb_offset == 0)
+		VB2_DIE("gbb_offset is not initialized\n");
 	return (struct vb2_gbb_header *)((void *)sd + sd->gbb_offset);
 }
 
@@ -530,9 +532,7 @@ int vb2api_diagnostic_ui_enabled(struct vb2_context *ctx)
 enum vb2_dev_default_boot_target vb2api_get_dev_default_boot_target(
 	struct vb2_context *ctx)
 {
-	struct vb2_gbb_header *gbb = vb2_get_gbb(ctx);
-
-	if (gbb->flags & VB2_GBB_FLAG_DEFAULT_DEV_BOOT_ALTFW)
+	if (vb2api_gbb_get_flags(ctx) & VB2_GBB_FLAG_DEFAULT_DEV_BOOT_ALTFW)
 		return VB2_DEV_DEFAULT_BOOT_TARGET_ALTFW;
 
 	switch (vb2_nv_get(ctx, VB2_NV_DEV_DEFAULT_BOOT)) {
@@ -608,7 +608,7 @@ char *vb2api_get_debug_info(struct vb2_context *ctx)
 	uint32_t used = 0;
 
 	struct vb2_shared_data *sd = vb2_get_sd(ctx);
-	struct vb2_gbb_header *gbb = vb2_get_gbb(ctx);
+	struct vb2_gbb_header *gbb = NULL;
 	struct vb2_workbuf wb;
 	char sha1sum[VB2_SHA1_DIGEST_SIZE * 2 + 1];
 
@@ -621,8 +621,14 @@ char *vb2api_get_debug_info(struct vb2_context *ctx)
 
 	vb2_workbuf_from_ctx(ctx, &wb);
 
+	if (sd->gbb_offset == 0) {
+		DEBUG_INFO_APPEND("GBB: {INVALID}");
+	} else {
+		gbb = vb2_get_gbb(ctx);
+	}
+
 	/* Add hardware ID */
-	{
+	if (gbb) {
 		char hwid[VB2_GBB_HWID_MAX_SIZE];
 		uint32_t size = sizeof(hwid);
 		rv = vb2api_gbb_read_hwid(ctx, hwid, &size);
@@ -674,10 +680,12 @@ char *vb2api_get_debug_info(struct vb2_context *ctx)
 			  sd->fw_version_secdata, sd->kernel_version_secdata);
 
 	/* Add GBB flags */
-	DEBUG_INFO_APPEND("\ngbb.flags: %#.8x", gbb->flags);
+	if (gbb) {
+		DEBUG_INFO_APPEND("\ngbb.flags: %#.8x", gbb->flags);
+	}
 
 	/* Add sha1sum for Root & Recovery keys */
-	{
+	if (gbb) {
 		struct vb2_packed_key *key;
 		struct vb2_workbuf wblocal = wb;
 		rv = vb2_gbb_read_root_key(ctx, &key, NULL, &wblocal);
@@ -687,7 +695,7 @@ char *vb2api_get_debug_info(struct vb2_context *ctx)
 		}
 	}
 
-	{
+	if (gbb) {
 		struct vb2_packed_key *key;
 		struct vb2_workbuf wblocal = wb;
 		rv = vb2_gbb_read_recovery_key(ctx, &key, NULL, &wblocal);
@@ -713,7 +721,6 @@ char *vb2api_get_debug_info(struct vb2_context *ctx)
 void vb2_set_boot_mode(struct vb2_context *ctx)
 {
 	struct vb2_shared_data *sd = vb2_get_sd(ctx);
-	struct vb2_gbb_header *gbb = vb2_get_gbb(ctx);
 
 	/* Cast boot mode to non-constant and assign */
 	enum vb2_boot_mode *boot_mode = (enum vb2_boot_mode *)&ctx->boot_mode;
@@ -730,7 +737,8 @@ void vb2_set_boot_mode(struct vb2_context *ctx)
 	    (ctx->flags & VB2_CONTEXT_EC_TRUSTED)) {
 		*boot_mode = VB2_BOOT_MODE_MANUAL_RECOVERY;
 	} else if (sd->recovery_reason) {
-		if (gbb->flags & VB2_GBB_FLAG_FORCE_MANUAL_RECOVERY)
+		vb2_gbb_flags_t gbb_flags = vb2api_gbb_get_flags(ctx);
+		if (gbb_flags & VB2_GBB_FLAG_FORCE_MANUAL_RECOVERY)
 			*boot_mode = VB2_BOOT_MODE_MANUAL_RECOVERY;
 		else
 			*boot_mode = VB2_BOOT_MODE_BROKEN_SCREEN;
diff --git a/tests/vb2_gbb_init_tests.c b/tests/vb2_gbb_init_tests.c
new file mode 100644
index 00000000..9a3989c0
--- /dev/null
+++ b/tests/vb2_gbb_init_tests.c
@@ -0,0 +1,26 @@
+/* Copyright 2022 The ChromiumOS Authors.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ *
+ * Test that vb2_get_gbb aborts if gbb_offset is not initialized. This
+ * is in a separate file from vb2_gbb_tests so that vb2_get_gbb is not
+ * mocked.
+ */
+
+#include "2common.h"
+#include "common/tests.h"
+
+static void test_abort_if_gbb_uninit(void) {
+	struct vb2_context *ctx;
+	uint8_t workbuf[VB2_KERNEL_WORKBUF_RECOMMENDED_SIZE]
+		__attribute__((aligned(VB2_WORKBUF_ALIGN)));
+	TEST_SUCC(vb2api_init(workbuf, sizeof(workbuf), &ctx),
+		  "vb2api_init failed");
+	TEST_ABORT(vb2_get_gbb(ctx), "gbb_offset is not initialized");
+}
+
+int main(int argc, char *argv[])
+{
+	test_abort_if_gbb_uninit();
+	return gTestSuccess ? 0 : 255;
+}