vboot: remove some vboot1 SharedData-related functions

Remove VbSharedDataInit, VbSharedDataReserve,
VbSharedDataSetKernelKey, along with their corresponding tests.

In tests/verify_kernel and utility/load_kernel_test, just copy the
kernel subkey manually for now, until we update LoadKernel to
retrieve the key from vboot2 workbuf instead.

BUG=b:124141368, chromium:968464
TEST=make clean && make runtests
BRANCH=none

Change-Id: I7055be10f0d7a38502fd8108f0bc93c5b78f06c9
Signed-off-by: Joel Kitching <kitching@google.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/1844598
Reviewed-by: Joel Kitching <kitching@chromium.org>
Commit-Queue: Joel Kitching <kitching@chromium.org>
Tested-by: Joel Kitching <kitching@chromium.org>

16 files changed, 46 insertions, 215 deletions

diff --git a/Makefile b/Makefile
index 471ba922..26cfc6d6 100644
--- a/Makefile
+++ b/Makefile
@@ -347,7 +347,6 @@ FWLIB_SRCS = \
 	firmware/lib/vboot_api_kernel.c \
 	firmware/lib/vboot_audio.c \
 	firmware/lib/vboot_common.c \
-	firmware/lib/vboot_common_init.c \
 	firmware/lib/vboot_display.c \
 	firmware/lib/vboot_kernel.c \
 	firmware/lib/vboot_ui.c \
diff --git a/firmware/include/vboot_struct.h b/firmware/include/vboot_struct.h
index 849d4b2f..9ed80dd2 100644
--- a/firmware/include/vboot_struct.h
+++ b/firmware/include/vboot_struct.h
@@ -317,6 +317,13 @@ typedef struct VbSharedDataHeader {
 #define VB_SHARED_DATA_HEADER_SIZE_V1 1072
 #define VB_SHARED_DATA_HEADER_SIZE_V2 1096
 
+_Static_assert(VB_SHARED_DATA_HEADER_SIZE_V1
+	       == offsetof(VbSharedDataHeader, recovery_reason),
+	       "VB_SHARED_DATA_HEADER_SIZE_V1 incorrect");
+
+_Static_assert(VB_SHARED_DATA_HEADER_SIZE_V2 == sizeof(VbSharedDataHeader),
+	       "VB_SHARED_DATA_HEADER_SIZE_V2 incorrect");
+
 #define VB_SHARED_DATA_VERSION 2  /* Version for struct_version */
 
 #ifdef __cplusplus
diff --git a/firmware/lib/include/vboot_common.h b/firmware/lib/include/vboot_common.h
index b430fa15..6851f2f0 100644
--- a/firmware/lib/include/vboot_common.h
+++ b/firmware/lib/include/vboot_common.h
@@ -33,25 +33,5 @@ int PublicKeyCopy(struct vb2_packed_key *dest,
  */
 vb2_error_t VerifyVmlinuzInsideKBlob(uint64_t kblob, uint64_t kblob_size,
 				     uint64_t header, uint64_t header_size);
-/**
- * Initialize a verified boot shared data structure.
- *
- * Returns 0 if success, non-zero if error.
- */
-vb2_error_t VbSharedDataInit(VbSharedDataHeader *header, uint64_t size);
-
-/**
- * Reserve [size] bytes of the shared data area.  Returns the offset of the
- * reserved data from the start of the shared data buffer, or 0 if error.
- */
-uint64_t VbSharedDataReserve(VbSharedDataHeader *header, uint64_t size);
-
-/**
- * Copy the kernel subkey into the shared data.
- *
- * Returns 0 if success, non-zero if error.
- */
-vb2_error_t VbSharedDataSetKernelKey(VbSharedDataHeader *header,
-				     const struct vb2_packed_key *src);
 
 #endif  /* VBOOT_REFERENCE_VBOOT_COMMON_H_ */
diff --git a/firmware/lib/vboot_common.c b/firmware/lib/vboot_common.c
index d062477f..f7e83803 100644
--- a/firmware/lib/vboot_common.c
+++ b/firmware/lib/vboot_common.c
@@ -51,50 +51,3 @@ vb2_error_t VerifyVmlinuzInsideKBlob(uint64_t kblob, uint64_t kblob_size,
 
 	return VB2_SUCCESS;
 }
-
-uint64_t VbSharedDataReserve(VbSharedDataHeader *header, uint64_t size)
-{
-	if (!header || size > header->data_size - header->data_used) {
-		VB2_DEBUG("VbSharedData buffer out of space.\n");
-		return 0;  /* Not initialized, or not enough space left. */
-	}
-
-	uint64_t offs = header->data_used;
-	VB2_DEBUG("VbSharedDataReserve %d bytes at %d\n", (int)size, (int)offs);
-
-	header->data_used += size;
-	return offs;
-}
-
-vb2_error_t VbSharedDataSetKernelKey(VbSharedDataHeader *header,
-				     const struct vb2_packed_key *src)
-{
-	struct vb2_packed_key *kdest;
-
-	if (!header)
-		return VBOOT_SHARED_DATA_INVALID;
-	if (!src)
-		return VBOOT_PUBLIC_KEY_INVALID;
-
-	kdest = &header->kernel_subkey;
-
-	VB2_DEBUG("Saving kernel subkey to shared data: size %d, algo %d\n",
-		  vb2_rsa_sig_size(vb2_crypto_to_signature(src->algorithm)),
-		  (int)src->algorithm);
-
-	/* Attempt to allocate space for key, if it hasn't been allocated yet */
-	if (!header->kernel_subkey_data_offset) {
-		header->kernel_subkey_data_offset =
-			VbSharedDataReserve(header, src->key_size);
-		if (!header->kernel_subkey_data_offset)
-			return VBOOT_SHARED_DATA_INVALID;
-		header->kernel_subkey_data_size = src->key_size;
-	}
-
-	/* Copy the kernel sign key blob into the destination buffer */
-	PublicKeyInit(kdest,
-		      (uint8_t *)header + header->kernel_subkey_data_offset,
-		      header->kernel_subkey_data_size);
-
-	return PublicKeyCopy(kdest, src);
-}
diff --git a/firmware/lib/vboot_common_init.c b/firmware/lib/vboot_common_init.c
deleted file mode 100644
index b370fa3a..00000000
--- a/firmware/lib/vboot_common_init.c
+++ /dev/null
@@ -1,45 +0,0 @@
-/* Copyright (c) 2013 The Chromium OS Authors. All rights reserved.
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- *
- * Common functions between firmware and kernel verified boot.
- * (Firmware portion)
- */
-
-#include "2common.h"
-#include "2sysincludes.h"
-#include "utility.h"
-#include "vboot_api.h"
-#include "vboot_common.h"
-
-vb2_error_t VbSharedDataInit(VbSharedDataHeader *header, uint64_t size)
-{
-	VB2_DEBUG("VbSharedDataInit, %d bytes, header %d bytes\n", (int)size,
-		  (int)sizeof(VbSharedDataHeader));
-
-	if (size < sizeof(VbSharedDataHeader)) {
-		VB2_DEBUG("Not enough data for header.\n");
-		return VBOOT_SHARED_DATA_INVALID;
-	}
-	if (size < VB_SHARED_DATA_MIN_SIZE) {
-		VB2_DEBUG("Shared data buffer too small.\n");
-		return VBOOT_SHARED_DATA_INVALID;
-	}
-
-	if (!header)
-		return VBOOT_SHARED_DATA_INVALID;
-
-	/* Zero the header */
-	memset(header, 0, sizeof(VbSharedDataHeader));
-
-	/* Initialize fields */
-	header->magic = VB_SHARED_DATA_MAGIC;
-	header->struct_version = VB_SHARED_DATA_VERSION;
-	header->struct_size = sizeof(VbSharedDataHeader);
-	header->data_size = size;
-	header->data_used = sizeof(VbSharedDataHeader);
-	header->firmware_index = 0xFF;
-
-	/* Success */
-	return VB2_SUCCESS;
-}
diff --git a/tests/vb2_auxfw_sync_tests.c b/tests/vb2_auxfw_sync_tests.c
index 1b211b86..470c2d2a 100644
--- a/tests/vb2_auxfw_sync_tests.c
+++ b/tests/vb2_auxfw_sync_tests.c
@@ -59,7 +59,6 @@ static void ResetMocks(void)
 	memset(&gbb, 0, sizeof(gbb));
 
 	memset(&shared_data, 0, sizeof(shared_data));
-	VbSharedDataInit(shared, sizeof(shared_data));
 
 	memset(screens_displayed, 0, sizeof(screens_displayed));
 	screens_count = 0;
diff --git a/tests/vb2_ec_sync_tests.c b/tests/vb2_ec_sync_tests.c
index 11926111..259f6479 100644
--- a/tests/vb2_ec_sync_tests.c
+++ b/tests/vb2_ec_sync_tests.c
@@ -14,7 +14,6 @@
 #include "secdata_tpm.h"
 #include "test_common.h"
 #include "vboot_audio.h"
-#include "vboot_common.h"
 #include "vboot_display.h"
 #include "vboot_kernel.h"
 #include "vboot_struct.h"
@@ -69,7 +68,6 @@ static void ResetMocks(void)
 	memset(&gbb, 0, sizeof(gbb));
 
 	memset(&shared_data, 0, sizeof(shared_data));
-	VbSharedDataInit(shared, sizeof(shared_data));
 
 	mock_in_rw = 0;
 	ec_ro_protected = 0;
diff --git a/tests/vboot_api_devmode_tests.c b/tests/vboot_api_devmode_tests.c
index 36983d1f..5754d8ff 100644
--- a/tests/vboot_api_devmode_tests.c
+++ b/tests/vboot_api_devmode_tests.c
@@ -20,7 +20,6 @@
 #include "load_kernel_fw.h"
 #include "secdata_tpm.h"
 #include "test_common.h"
-#include "vboot_common.h"
 #include "vboot_display.h"
 #include "vboot_kernel.h"
 #include "vboot_struct.h"
@@ -128,7 +127,6 @@ static void ResetMocks(void)
 	memset(&gbb, 0, sizeof(gbb));
 
 	memset(&shared_data, 0, sizeof(shared_data));
-	VbSharedDataInit(shared, sizeof(shared_data));
 	shared->fw_keyblock_flags = 0xABCDE0;
 
 	current_ticks = 0;
diff --git a/tests/vboot_api_kernel2_tests.c b/tests/vboot_api_kernel2_tests.c
index 55c7d70c..2d490681 100644
--- a/tests/vboot_api_kernel2_tests.c
+++ b/tests/vboot_api_kernel2_tests.c
@@ -16,7 +16,6 @@
 #include "test_common.h"
 #include "tss_constants.h"
 #include "vboot_audio.h"
-#include "vboot_common.h"
 #include "vboot_display.h"
 #include "vboot_kernel.h"
 #include "vboot_struct.h"
@@ -88,7 +87,6 @@ static void ResetMocks(void)
 	vb2_init_ui();
 
 	memset(&shared_data, 0, sizeof(shared_data));
-	VbSharedDataInit(shared, sizeof(shared_data));
 
 	memset(&lkp, 0, sizeof(lkp));
 
diff --git a/tests/vboot_api_kernel4_tests.c b/tests/vboot_api_kernel4_tests.c
index c0a10727..80c8099a 100644
--- a/tests/vboot_api_kernel4_tests.c
+++ b/tests/vboot_api_kernel4_tests.c
@@ -18,7 +18,6 @@
 #include "tlcl.h"
 #include "tss_constants.h"
 #include "vboot_audio.h"
-#include "vboot_common.h"
 #include "vboot_kernel.h"
 #include "vboot_struct.h"
 #include "vboot_test.h"
@@ -68,7 +67,6 @@ static void ResetMocks(void)
 	commit_data_called = 0;
 
 	memset(&shared_data, 0, sizeof(shared_data));
-	VbSharedDataInit(shared, sizeof(shared_data));
 
 	memset(&fwmp_buf, 0, sizeof(fwmp_buf));
 	fwmp_read_retval = TPM_SUCCESS;
diff --git a/tests/vboot_common_tests.c b/tests/vboot_common_tests.c
index b41020ab..50e2bb59 100644
--- a/tests/vboot_common_tests.c
+++ b/tests/vboot_common_tests.c
@@ -15,21 +15,6 @@
 #include "utility.h"
 #include "vboot_common.h"
 
-/*
- * Test struct packing for vboot_struct.h structs which are passed between
- * firmware and OS, or passed between different phases of firmware.
- */
-static void StructPackingTest(void)
-{
-	TEST_EQ(VB_SHARED_DATA_HEADER_SIZE_V1,
-		(long)&((VbSharedDataHeader*)NULL)->recovery_reason,
-		"sizeof(VbSharedDataHeader) V1");
-
-	TEST_EQ(VB_SHARED_DATA_HEADER_SIZE_V2,
-		sizeof(VbSharedDataHeader),
-		"sizeof(VbSharedDataHeader) V2");
-}
-
 /* Public key utility functions */
 static void PublicKeyTest(void)
 {
@@ -79,54 +64,9 @@ static void PublicKeyTest(void)
 		"PublicKeyCopy data");
 }
 
-/* VbSharedData utility tests */
-static void VbSharedDataTest(void)
-{
-	uint8_t buf[VB_SHARED_DATA_MIN_SIZE + 1];
-	VbSharedDataHeader* d = (VbSharedDataHeader*)buf;
-
-	TEST_NEQ(VB2_SUCCESS,
-		 VbSharedDataInit(d, sizeof(VbSharedDataHeader) - 1),
-		 "VbSharedDataInit too small");
-	TEST_NEQ(VB2_SUCCESS,
-		 VbSharedDataInit(d, VB_SHARED_DATA_MIN_SIZE - 1),
-		 "VbSharedDataInit too small 2");
-	TEST_NEQ(VB2_SUCCESS,
-		 VbSharedDataInit(NULL, VB_SHARED_DATA_MIN_SIZE),
-		 "VbSharedDataInit null");
-
-	memset(buf, 0x68, sizeof(buf));
-	TEST_EQ(VB2_SUCCESS, VbSharedDataInit(d, VB_SHARED_DATA_MIN_SIZE),
-		"VbSharedDataInit");
-
-	/* Check fields that should have been initialized */
-	TEST_EQ(d->magic, VB_SHARED_DATA_MAGIC, "VbSharedDataInit magic");
-	TEST_EQ(d->struct_version, VB_SHARED_DATA_VERSION,
-		"VbSharedDataInit version");
-	TEST_EQ(d->struct_size, sizeof(VbSharedDataHeader),
-		"VbSharedDataInit struct_size");
-	TEST_EQ(d->data_size, VB_SHARED_DATA_MIN_SIZE,
-		"VbSharedDataInit data_size");
-	TEST_EQ(d->data_used, d->struct_size, "VbSharedDataInit data_used");
-	TEST_EQ(d->firmware_index, 0xFF, "VbSharedDataInit firmware index");
-
-	/* Sample some other fields to make sure they were zeroed */
-	TEST_EQ(d->flags, 0, "VbSharedDataInit firmware flags");
-	TEST_EQ(d->lk_call_count, 0, "VbSharedDataInit lk_call_count");
-	TEST_EQ(d->kernel_version_lowest, 0,
-		"VbSharedDataInit kernel_version_lowest");
-
-	TEST_EQ(VBOOT_SHARED_DATA_INVALID, VbSharedDataSetKernelKey(NULL, NULL),
-		"VbSharedDataSetKernelKey sd null");
-	TEST_EQ(VBOOT_PUBLIC_KEY_INVALID, VbSharedDataSetKernelKey(d, NULL),
-		"VbSharedDataSetKernelKey pubkey null");
-}
-
 int main(int argc, char* argv[])
 {
-	StructPackingTest();
 	PublicKeyTest();
-	VbSharedDataTest();
 
 	return gTestSuccess ? 0 : 255;
 }
diff --git a/tests/vboot_detach_menu_tests.c b/tests/vboot_detach_menu_tests.c
index 3db3da9d..c2ed73a4 100644
--- a/tests/vboot_detach_menu_tests.c
+++ b/tests/vboot_detach_menu_tests.c
@@ -17,7 +17,6 @@
 #include "tss_constants.h"
 #include "vboot_api.h"
 #include "vboot_audio.h"
-#include "vboot_common.h"
 #include "vboot_display.h"
 #include "vboot_kernel.h"
 #include "vboot_struct.h"
@@ -64,7 +63,6 @@ static int vbexaltfwmask_called;
 static void ResetMocks(void)
 {
 	memset(&shared_data, 0, sizeof(shared_data));
-	VbSharedDataInit(shared, sizeof(shared_data));
 	shared->flags = VBSD_BOOT_FIRMWARE_VBOOT2;
 
 	memset(&lkp, 0, sizeof(lkp));
diff --git a/tests/vboot_display_tests.c b/tests/vboot_display_tests.c
index 7565a196..634462e3 100644
--- a/tests/vboot_display_tests.c
+++ b/tests/vboot_display_tests.c
@@ -18,7 +18,6 @@
 #include "host_common.h"
 #include "secdata_tpm.h"
 #include "test_common.h"
-#include "vboot_common.h"
 #include "vboot_display.h"
 #include "vboot_kernel.h"
 
@@ -47,7 +46,6 @@ static void ResetMocks(void)
 	sd->vbsd = shared;
 
 	memset(&shared_data, 0, sizeof(shared_data));
-	VbSharedDataInit(shared, sizeof(shared_data));
 
 	*debug_info = 0;
 }
diff --git a/tests/vboot_kernel_tests.c b/tests/vboot_kernel_tests.c
index fdbc7575..9063aeb6 100644
--- a/tests/vboot_kernel_tests.c
+++ b/tests/vboot_kernel_tests.c
@@ -23,7 +23,6 @@
 #include "test_common.h"
 #include "vb2_common.h"
 #include "vboot_api.h"
-#include "vboot_common.h"
 #include "vboot_kernel.h"
 
 #define LOGCALL(fmt, args...) sprintf(call_log + strlen(call_log), fmt, ##args)
@@ -140,7 +139,6 @@ static void ResetMocks(void)
 	gbb.flags = 0;
 
 	memset(&shared_data, 0, sizeof(shared_data));
-	VbSharedDataInit(shared, sizeof(shared_data));
 	shared->kernel_version_tpm = 0x20001;
 
 	memset(&lkp, 0, sizeof(lkp));
diff --git a/tests/verify_kernel.c b/tests/verify_kernel.c
index 051e629d..ddb1fe30 100644
--- a/tests/verify_kernel.c
+++ b/tests/verify_kernel.c
@@ -26,7 +26,8 @@ static struct vb2_shared_data *sd;
 
 static uint8_t *diskbuf;
 
-static uint8_t shared_data[VB_SHARED_DATA_MIN_SIZE];
+static uint8_t shared_data[VB_SHARED_DATA_MIN_SIZE]
+	__attribute__((aligned(VB2_WORKBUF_ALIGN)));
 static VbSharedDataHeader *shared = (VbSharedDataHeader *)shared_data;
 
 static LoadKernelParams params;
@@ -91,11 +92,6 @@ int main(int argc, char *argv[])
 		return 1;
 	}
 
-	/* Set up shared data blob */
-	VbSharedDataInit(shared, sizeof(shared_data));
-	VbSharedDataSetKernelKey(shared, kernkey);
-	/* TODO: optional TPM current kernel version */
-
 	/* Set up params */
 	params.disk_handle = (VbExDiskHandle_t)1;
 	params.bytes_per_lba = 512;
@@ -116,9 +112,21 @@ int main(int argc, char *argv[])
 		fprintf(stderr, "Can't initialize workbuf\n");
 		return 1;
 	}
+	memset(&shared_data, 0, sizeof(shared_data));
 	sd = vb2_get_sd(ctx);
 	sd->vbsd = shared;
 
+	/* Copy kernel subkey to VBSD */
+	struct vb2_packed_key *dst = (struct vb2_packed_key *)
+		(shared_data + vb2_wb_round_up(sizeof(VbSharedDataHeader)));
+	shared->kernel_subkey.key_offset =
+		(uintptr_t)dst - (uintptr_t)&shared->kernel_subkey;
+	shared->kernel_subkey.key_size = kernkey->key_size;
+	shared->kernel_subkey.algorithm = kernkey->algorithm;
+	shared->kernel_subkey.key_version = kernkey->key_version;
+	memcpy(vb2_packed_key_data_mutable(dst), vb2_packed_key_data(kernkey),
+	       kernkey->key_size);
+
 	/*
 	 * LoadKernel() cares only about VBNV_DEV_BOOT_SIGNED_ONLY, and only in
 	 * dev mode.  So just use defaults for nv storage.
diff --git a/utility/load_kernel_test.c b/utility/load_kernel_test.c
index 540d4382..3023e219 100644
--- a/utility/load_kernel_test.c
+++ b/utility/load_kernel_test.c
@@ -26,6 +26,10 @@ static uint8_t workbuf[VB2_KERNEL_WORKBUF_RECOMMENDED_SIZE];
 static struct vb2_context *ctx;
 static struct vb2_shared_data *sd;
 
+static uint8_t shared_data[VB_SHARED_DATA_MIN_SIZE]
+	__attribute__((aligned(VB2_WORKBUF_ALIGN)));
+static VbSharedDataHeader *shared = (VbSharedDataHeader *)shared_data;
+
 /* Global variables for stub functions */
 static LoadKernelParams lkp;
 static FILE *image_file = NULL;
@@ -90,8 +94,7 @@ int main(int argc, char* argv[])
 {
 	const char* image_name;
 	uint64_t key_size = 0;
-	uint8_t* key_blob = NULL;
-	VbSharedDataHeader* shared;
+	struct vb2_packed_key *key_blob = NULL;
 	struct vb2_gbb_header* gbb;
 	vb2_error_t rv;
 	int c, argsleft;
@@ -152,7 +155,8 @@ int main(int argc, char* argv[])
 
 	/* Read header signing key blob */
 	if (argsleft > 1) {
-		key_blob = ReadFile(argv[optind+1], &key_size);
+		key_blob = (struct vb2_packed_key *)
+			ReadFile(argv[optind+1], &key_size);
 		if (!key_blob) {
 			fprintf(stderr, "Unable to read key file %s\n",
 				argv[optind+1]);
@@ -186,24 +190,6 @@ int main(int argc, char* argv[])
 		gbb->recovery_key_size = key_size;
 	}
 
-	/* Initialize the shared data area */
-	shared = (VbSharedDataHeader*)malloc(VB_SHARED_DATA_REC_SIZE);
-	if (0 != VbSharedDataInit(shared, VB_SHARED_DATA_REC_SIZE)) {
-		fprintf(stderr, "Unable to init shared data\n");
-		return 1;
-	}
-	/* Copy in the key blob, if any */
-	if (key_blob) {
-		if (0 != VbSharedDataSetKernelKey(
-				shared, (struct vb2_packed_key *)key_blob)) {
-			fprintf(stderr, "Unable to set key in shared data\n");
-			return 1;
-		}
-	}
-
-	/* Free the key blob, now that we're done with it */
-	free(key_blob);
-
 	printf("bootflags = %d\n", boot_flags);
 	lkp.boot_flags = boot_flags;
 
@@ -233,9 +219,27 @@ int main(int argc, char* argv[])
 		fprintf(stderr, "Can't initialize workbuf\n");
 		return 1;
 	}
+	memset(&shared_data, 0, sizeof(shared_data));
 	sd = vb2_get_sd(ctx);
 	sd->vbsd = shared;
 
+	/* Copy kernel subkey to VBSD, if any */
+	if (key_blob) {
+		struct vb2_packed_key *dst = (struct vb2_packed_key *)
+			(shared_data +
+			 vb2_wb_round_up(sizeof(VbSharedDataHeader)));
+		shared->kernel_subkey.key_offset =
+			(uintptr_t)dst - (uintptr_t)&shared->kernel_subkey;
+		shared->kernel_subkey.key_size = key_blob->key_size;
+		shared->kernel_subkey.algorithm = key_blob->algorithm;
+		shared->kernel_subkey.key_version = key_blob->key_version;
+		memcpy(vb2_packed_key_data_mutable(dst),
+		       vb2_packed_key_data(key_blob), key_blob->key_size);
+	}
+
+	/* Free the key blob, now that we're done with it */
+	free(key_blob);
+
 	/* No need to initialize ctx->nvdata[]; defaults are fine */
 	/* TODO(chromium:441893): support dev-mode flag and external gpt flag */
 	if (boot_flags & BOOT_FLAG_RECOVERY)