crossystem: Add a fake workbuffer for the fake context

crossystem functions just make up a fake vb2_context for calling into
things like vb2_nv_init(), but that function actually accesses
vb2_shared_data as well. This used to work because vb2_get_sd() would
return NULL in that case and vb2_nv_init() actually checks for that,
but with the persistent context model this is no longer possible and
making up directly allocated contexts is always illegal.

This patch adds a small fake workbuffer to the fake context so we can
have real backing storage for shared data. (This might not be the final
way we want to fix it but should work as a quick band-aid over the
crashes.)

Also remove the now pointless (sd == NULL) checks from vb2_nv_init().

BRANCH=None
BUG=chromium:1024732
TEST=make runtests

Change-Id: I91247013f092bbfc41cf1974b82cf70a29fa4734
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/1917486
Tested-by: Brian Norris <briannorris@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Commit-Queue: Brian Norris <briannorris@chromium.org>

3 files changed, 31 insertions, 29 deletions

diff --git a/Makefile b/Makefile
index 3519b7e7..e857875f 100644
--- a/Makefile
+++ b/Makefile
@@ -481,6 +481,7 @@ HOSTLIB_SRCS = \
 	cgpt/cgpt_prioritize.c \
 	cgpt/cgpt_show.c \
 	firmware/2lib/2common.c \
+	firmware/2lib/2context.c \
 	firmware/2lib/2crc8.c \
 	firmware/2lib/2hmac.c \
 	firmware/2lib/2nvstorage.c \
diff --git a/firmware/2lib/2nvstorage.c b/firmware/2lib/2nvstorage.c
index e3846d30..7af83762 100644
--- a/firmware/2lib/2nvstorage.c
+++ b/firmware/2lib/2nvstorage.c
@@ -76,24 +76,13 @@ void vb2_nv_init(struct vb2_context *ctx)
 		/* Regenerate CRC */
 		vb2_nv_regen_crc(ctx);
 
-		/*
-		 * Set status flag.
-		 *
-		 * Note that early in some calling sequences, shared data may
-		 * not be available.  For example, if there is an error
-		 * allocating the context work buffer, and we're trying to
-		 * initialize non-volatile storage so we can write a recovery
-		 * request.  In that case, sd will be NULL.  So while we don't
-		 * usually need to check for that in other library functions,
-		 * here we do.
-		 */
-		if (sd)
-			sd->status |= VB2_SD_STATUS_NV_REINIT;
+		/* Set status flag. */
+		sd->status |= VB2_SD_STATUS_NV_REINIT;
+
 		/* TODO: unit test for status flag being set */
 	}
 
-	if (sd)
-		sd->status |= VB2_SD_STATUS_NV_INIT;
+	sd->status |= VB2_SD_STATUS_NV_INIT;
 }
 
 /* Macro for vb2_nv_get() single-bit settings to reduce duplicate code. */
diff --git a/host/lib/crossystem.c b/host/lib/crossystem.c
index 3a418939..be8203e1 100644
--- a/host/lib/crossystem.c
+++ b/host/lib/crossystem.c
@@ -94,26 +94,39 @@ int FwidStartsWith(const char *start)
 	return 0 == strncmp(fwid, start, strlen(start));
 }
 
+static struct vb2_context *get_fake_context(void)
+{
+	static uint8_t fake_workbuf[sizeof(struct vb2_shared_data) + 16]
+		__attribute__((aligned(VB2_WORKBUF_ALIGN)));
+	static struct vb2_context *fake_ctx;
+
+	if (fake_ctx)
+		return fake_ctx;
+
+	vb2api_init(fake_workbuf, sizeof(fake_workbuf), &fake_ctx);
+
+	return fake_ctx;
+}
+
 static int vnc_read;
 
 int vb2_get_nv_storage(enum vb2_nv_param param)
 {
 	VbSharedDataHeader* sh = VbSharedDataRead();
-	static struct vb2_context cached_ctx;
+	struct vb2_context *ctx = get_fake_context();
 
 	if (!sh)
 		return -1;
 
 	/* TODO: locking around NV access */
 	if (!vnc_read) {
-		memset(&cached_ctx, 0, sizeof(cached_ctx));
 		if (sh && sh->flags & VBSD_NVDATA_V2)
-			cached_ctx.flags |= VB2_CONTEXT_NVDATA_V2;
-		if (0 != vb2_read_nv_storage(&cached_ctx)) {
+			ctx->flags |= VB2_CONTEXT_NVDATA_V2;
+		if (0 != vb2_read_nv_storage(ctx)) {
 			free(sh);
 			return -1;
 		}
-		vb2_nv_init(&cached_ctx);
+		vb2_nv_init(ctx);
 
 		/* TODO: If vnc.raw_changed, attempt to reopen NVRAM for write
 		 * and save the new defaults.  If we're able to, log. */
@@ -122,31 +135,30 @@ int vb2_get_nv_storage(enum vb2_nv_param param)
 	}
 
 	free(sh);
-	return (int)vb2_nv_get(&cached_ctx, param);
+	return (int)vb2_nv_get(ctx, param);
 }
 
 int vb2_set_nv_storage(enum vb2_nv_param param, int value)
 {
 	VbSharedDataHeader* sh = VbSharedDataRead();
-	struct vb2_context ctx;
+	struct vb2_context *ctx = get_fake_context();
 
 	if (!sh)
 		return -1;
 
 	/* TODO: locking around NV access */
-	memset(&ctx, 0, sizeof(ctx));
 	if (sh && sh->flags & VBSD_NVDATA_V2)
-		ctx.flags |= VB2_CONTEXT_NVDATA_V2;
-	if (0 != vb2_read_nv_storage(&ctx)) {
+		ctx->flags |= VB2_CONTEXT_NVDATA_V2;
+	if (0 != vb2_read_nv_storage(ctx)) {
 		free(sh);
 		return -1;
 	}
-	vb2_nv_init(&ctx);
-	vb2_nv_set(&ctx, param, (uint32_t)value);
+	vb2_nv_init(ctx);
+	vb2_nv_set(ctx, param, (uint32_t)value);
 
-	if (ctx.flags & VB2_CONTEXT_NVDATA_CHANGED) {
+	if (ctx->flags & VB2_CONTEXT_NVDATA_CHANGED) {
 		vnc_read = 0;
-		if (0 != vb2_write_nv_storage(&ctx)) {
+		if (0 != vb2_write_nv_storage(ctx)) {
 			free(sh);
 			return -1;
 		}