From 7e21698e42dba31cc3e3c6b58a31bd050d3698ac Mon Sep 17 00:00:00 2001 From: Julius Werner Date: Fri, 29 May 2015 16:56:20 -0700 Subject: vboot2: secdata: Check struct_version on initialization This patch reintroduces a vb2_secdata->struct_version check similar to the one that was removed in CL:244846. The CRC is not a reliable way to detect zeroed buffers, so this check helps vboot fail earlier and more clearly in certain situations. BRANCH=kitty,smaug,storm,veyron BUG=chrome-os-partner:40778 TEST=make runtests. Rebooted Jerry with 'mem w 0xff7601b0 0xfdb9', saw that recovery reason was now 0x2b (VBNV_RECOVERY_VB2_SECDATA_INIT). Change-Id: Ic4376d127e6d14d4ef9c2f53c83090040ca4cb68 Signed-off-by: Julius Werner Reviewed-on: https://chromium-review.googlesource.com/274138 Reviewed-by: Randall Spangler Reviewed-by: Vadim Bendebury --- firmware/2lib/2secdata.c | 4 ++++ firmware/2lib/include/2return_codes.h | 4 ++-- tests/vb2_secdata_tests.c | 4 ++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/firmware/2lib/2secdata.c b/firmware/2lib/2secdata.c index 0fd6c522..3281f7c3 100644 --- a/firmware/2lib/2secdata.c +++ b/firmware/2lib/2secdata.c @@ -20,6 +20,10 @@ int vb2_secdata_check_crc(const struct vb2_context *ctx) if (sec->crc8 != vb2_crc8(sec, offsetof(struct vb2_secdata, crc8))) return VB2_ERROR_SECDATA_CRC; + /* CRC(<000...00>) is 0, so check version as well (should never be 0) */ + if (!sec->struct_version) + return VB2_ERROR_SECDATA_ZERO; + return VB2_SUCCESS; } diff --git a/firmware/2lib/include/2return_codes.h b/firmware/2lib/include/2return_codes.h index d05cd58b..81a59571 100644 --- a/firmware/2lib/include/2return_codes.h +++ b/firmware/2lib/include/2return_codes.h @@ -93,8 +93,8 @@ enum vb2_return_code { /* Bad CRC in vb2_secdata_check_crc() */ VB2_ERROR_SECDATA_CRC, - /* Bad struct version in vb2_secdata_init() */ - VB2_ERROR_SECDATA_VERSION, + /* Secdata is all zeroes (uninitialized) in vb2_secdata_check_crc() */ + VB2_ERROR_SECDATA_ZERO, /* Invalid param in vb2_secdata_get() */ VB2_ERROR_SECDATA_GET_PARAM, diff --git a/tests/vb2_secdata_tests.c b/tests/vb2_secdata_tests.c index 94f92fb7..b8ffafaa 100644 --- a/tests/vb2_secdata_tests.c +++ b/tests/vb2_secdata_tests.c @@ -46,6 +46,10 @@ static void secdata_test(void) TEST_EQ(vb2_secdata_init(&c), VB2_ERROR_SECDATA_CRC, "Init blank CRC"); + /* Ensure zeroed buffers are invalid (coreboot relies on this) */ + memset(c.secdata, 0, sizeof(c.secdata)); + TEST_EQ(vb2_secdata_init(&c), VB2_ERROR_SECDATA_ZERO, "Zeroed buffer"); + /* Create good data */ TEST_SUCC(vb2_secdata_create(&c), "Create"); TEST_SUCC(vb2_secdata_check_crc(&c), "Check created CRC"); -- cgit v1.2.1