vboot/vboot_kernel: store kparams pointer instead of copying data

Store kparams as a pointer in vboot_api_kernel.c, rather than a
full struct passed around as a "buffer copy".

This CL is part of a series to merge vboot1 and vboot2.0
kernel verification code; see b/181739551.

BUG=b:181739551
TEST=make clean && make runtests
BRANCH=none

Signed-off-by: Joel Kitching <kitching@google.com>
Change-Id: I7be7280761564c35e928bc947177b3fce61d6215
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2856360
Tested-by: Joel Kitching <kitching@chromium.org>
Reviewed-by: Yu-Ping Wu <yupingso@chromium.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Commit-Queue: Joel Kitching <kitching@chromium.org>

4 files changed, 21 insertions, 49 deletions

diff --git a/firmware/lib/include/vboot_test.h b/firmware/lib/include/vboot_test.h
index 41e8e5d9..fb1f52ad 100644
--- a/firmware/lib/include/vboot_test.h
+++ b/firmware/lib/include/vboot_test.h
@@ -12,6 +12,6 @@
  * vboot_api_kernel.c */
 
 struct VbSelectAndLoadKernelParams;
-struct VbSelectAndLoadKernelParams *VbApiKernelGetParams(void);
+struct VbSelectAndLoadKernelParams **VbApiKernelGetParamsPtr(void);
 
 #endif  /* VBOOT_REFERENCE_TEST_API_H_ */
diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c
index 93de040e..799424a2 100644
--- a/firmware/lib/vboot_api_kernel.c
+++ b/firmware/lib/vboot_api_kernel.c
@@ -21,13 +21,13 @@
 #include "vboot_test.h"
 
 /* Global variables */
-static VbSelectAndLoadKernelParams lkp;
+static VbSelectAndLoadKernelParams *kparams_ptr;
 
 #ifdef CHROMEOS_ENVIRONMENT
 /* Global variable accessor for unit tests */
-struct VbSelectAndLoadKernelParams *VbApiKernelGetParams(void)
+struct VbSelectAndLoadKernelParams **VbApiKernelGetParamsPtr(void)
 {
-	return &lkp;
+	return &kparams_ptr;
 }
 #endif
 
@@ -71,7 +71,11 @@ vb2_error_t VbTryLoadKernel(struct vb2_context *ctx, uint32_t disk_flags)
 	uint32_t disk_count = 0;
 	uint32_t i;
 
-	lkp.disk_handle = NULL;
+	/* TODO: Should have been set by VbSelectAndLoadKernel. Remove when
+	   this global is no longer needed. */
+	VB2_ASSERT(kparams_ptr);
+
+	kparams_ptr->disk_handle = NULL;
 
 	/* Find disks */
 	if (VB2_SUCCESS != VbExDiskGetInfo(&disk_info, &disk_count, disk_flags))
@@ -90,8 +94,9 @@ vb2_error_t VbTryLoadKernel(struct vb2_context *ctx, uint32_t disk_flags)
 			continue;
 		}
 
-		lkp.disk_handle = disk_info[i].handle;
-		vb2_error_t new_rv = LoadKernel(ctx, &lkp, &disk_info[i]);
+		kparams_ptr->disk_handle = disk_info[i].handle;
+		vb2_error_t new_rv = LoadKernel(ctx, kparams_ptr,
+						&disk_info[i]);
 		VB2_DEBUG("LoadKernel() = %#x\n", new_rv);
 
 		/* Stop now if we found a kernel. */
@@ -130,52 +135,20 @@ vb2_error_t VbTryLoadKernel(struct vb2_context *ctx, uint32_t disk_flags)
 	return rv;
 }
 
-static vb2_error_t vb2_kernel_init_kparams(struct vb2_context *ctx,
-					   VbSelectAndLoadKernelParams *kparams)
-{
-	/* Fill in params for calls to LoadKernel() */
-	memset(&lkp, 0, sizeof(lkp));
-	lkp.kernel_buffer = kparams->kernel_buffer;
-	lkp.kernel_buffer_size = kparams->kernel_buffer_size;
-
-	/* Clear output params in case we fail */
-	kparams->disk_handle = NULL;
-	kparams->partition_number = 0;
-	kparams->bootloader_address = 0;
-	kparams->bootloader_size = 0;
-	kparams->flags = 0;
-	memset(kparams->partition_guid, 0, sizeof(kparams->partition_guid));
-
-	return VB2_SUCCESS;
-}
-
-static void vb2_kernel_fill_kparams(struct vb2_context *ctx,
-				    VbSelectAndLoadKernelParams *kparams)
-{
-	/* Save disk parameters */
-	kparams->disk_handle = lkp.disk_handle;
-	kparams->partition_number = lkp.partition_number;
-	kparams->bootloader_address = lkp.bootloader_address;
-	kparams->bootloader_size = lkp.bootloader_size;
-	kparams->flags = lkp.flags;
-	kparams->kernel_buffer = lkp.kernel_buffer;
-	kparams->kernel_buffer_size = lkp.kernel_buffer_size;
-	memcpy(kparams->partition_guid, lkp.partition_guid,
-	       sizeof(kparams->partition_guid));
-}
-
 vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx,
 				  VbSelectAndLoadKernelParams *kparams)
 {
 	struct vb2_shared_data *sd = vb2_get_sd(ctx);
 	vb2_gbb_flags_t gbb_flags = vb2api_gbb_get_flags(ctx);
 
+	/* TODO: Send this argument through subsequent function calls, rather
+	   than relying on a global to pass it to VbTryLoadKernel. */
+	kparams_ptr = kparams;
+
 	/* Init nvstorage space. TODO(kitching): Remove once we add assertions
 	   to vb2_nv_get and vb2_nv_set. */
 	vb2_nv_init(ctx);
 
-	VB2_TRY(vb2_kernel_init_kparams(ctx, kparams));
-
 	VB2_TRY(vb2api_kernel_phase1(ctx));
 
 	VB2_DEBUG("GBB flags are %#x\n", gbb_flags);
@@ -262,6 +235,5 @@ vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx,
 		return VB2_ERROR_ESCAPE_NO_BOOT;
 	}
 
-	vb2_kernel_fill_kparams(ctx, kparams);
 	return VB2_SUCCESS;
 }
diff --git a/firmware/lib/vboot_kernel.c b/firmware/lib/vboot_kernel.c
index eb96a5c0..5ef60500 100644
--- a/firmware/lib/vboot_kernel.c
+++ b/firmware/lib/vboot_kernel.c
@@ -465,11 +465,8 @@ vb2_error_t LoadKernel(struct vb2_context *ctx,
 	uint32_t lowest_version = LOWEST_TPM_VERSION;
 	vb2_error_t rv;
 
-	/* Clear output params in case we fail */
+	/* Clear output params */
 	params->partition_number = 0;
-	params->bootloader_address = 0;
-	params->bootloader_size = 0;
-	params->flags = 0;
 
 	/* Read GPT data */
 	GptData gpt;
diff --git a/tests/vboot_api_kernel_tests.c b/tests/vboot_api_kernel_tests.c
index 9294fd7f..16933ea6 100644
--- a/tests/vboot_api_kernel_tests.c
+++ b/tests/vboot_api_kernel_tests.c
@@ -403,6 +403,7 @@ static uint32_t got_external_mismatch;
 static uint8_t workbuf[VB2_KERNEL_WORKBUF_RECOMMENDED_SIZE]
 	__attribute__((aligned(VB2_WORKBUF_ALIGN)));
 static struct vb2_context *ctx;
+static struct VbSelectAndLoadKernelParams kparams;
 
 /**
  * Reset mock data (for use before each test)
@@ -412,7 +413,9 @@ static void ResetMocks(int i)
 	TEST_SUCC(vb2api_init(workbuf, sizeof(workbuf), &ctx),
 		  "vb2api_init failed");
 
-	memset(VbApiKernelGetParams(), 0, sizeof(VbSelectAndLoadKernelParams));
+	memset(&kparams, 0, sizeof(VbSelectAndLoadKernelParams));
+	*VbApiKernelGetParamsPtr() = &kparams;
+
 
 	memset(&mock_disks, 0, sizeof(mock_disks));
 	load_kernel_calls = 0;