diff options
author | Jack Rosenthal <jrosenth@chromium.org> | 2020-09-09 17:47:21 -0600 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2020-09-10 20:03:26 +0000 |
commit | 176e01ded3bcefb6cb8baa984a158d42562bb1e9 (patch) | |
tree | fe038ca26e3111a1d65554f6b8a5417440aa9f36 | |
parent | 3dd8fe4c4e927fdb14a7a943911f6f118c56bc0a (diff) | |
download | vboot-176e01ded3bcefb6cb8baa984a158d42562bb1e9.tar.gz |
crossystem: allow last nvdata entry to be filled
Mosys used to have code (below), which led me to believe that we
always try and leave the last entry unfilled:
memset(blank, 0xff, VBNV_BLOCK_SIZE);
for (index = 0; index < len / VBNV_BLOCK_SIZE; index++) {
unsigned int offset = index * VBNV_BLOCK_SIZE;
if (!memcmp(blank, &data[offset], VBNV_BLOCK_SIZE))
break;
}
if (index == 0) {
lprintf(LOG_ERR, "VBNV is uninitialized\n");
return -1;
} else if (index >= len) { <---- SEE NOTE
lprintf(LOG_ERR, "VBNV is full\n"); <--- unreachable
return -1;
} else {
return index - 1;
}
The statement at "SEE NOTE" will always be false, so this code fooled
me to believe that we consider VBNV without a row of 0xFF*16 to be
empty.
And so I implemented and wrote unit tests for what I believed the
correct behavior to be :/
Anyway, this is causing us issues since AP firmware does not implement
it that way. So allow the last row to be filled.
BUG=chromium:1112578
BRANCH=none
TEST=unit tests
Signed-off-by: Jack Rosenthal <jrosenth@chromium.org>
Change-Id: Ib3da78eddef69a688d081cdb5391a25000dac9d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2402385
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
-rw-r--r-- | host/lib/crossystem.c | 6 | ||||
-rw-r--r-- | tests/vb2_host_nvdata_flashrom_tests.c | 6 |
2 files changed, 6 insertions, 6 deletions
diff --git a/host/lib/crossystem.c b/host/lib/crossystem.c index 63f20c42..59acc0d2 100644 --- a/host/lib/crossystem.c +++ b/host/lib/crossystem.c @@ -685,8 +685,8 @@ static int vb2_nv_index(const uint8_t *buf, uint32_t buf_sz, int vbnv_size) break; } - if (!index || index == buf_sz / vbnv_size) { - fprintf(stderr, "VBNV is either uninitialized or corrupted.\n"); + if (!index) { + fprintf(stderr, "VBNV is uninitialized.\n"); return -1; } @@ -737,7 +737,7 @@ int vb2_write_nv_storage_flashrom(struct vb2_context *ctx) } next_index = current_index + 1; - if ((next_index + 1) * vbnv_size == flash_size) { + if (next_index * vbnv_size == flash_size) { /* VBNV is full. Erase and write at beginning. */ memset(flash_buf, 0xff, flash_size); next_index = 0; diff --git a/tests/vb2_host_nvdata_flashrom_tests.c b/tests/vb2_host_nvdata_flashrom_tests.c index 7a147856..33b435d9 100644 --- a/tests/vb2_host_nvdata_flashrom_tests.c +++ b/tests/vb2_host_nvdata_flashrom_tests.c @@ -147,12 +147,12 @@ static void test_read_ok_full(void) reset_test_data(&ctx, sizeof(test_nvdata_16b)); - for (int entry = 0; entry < fake_flash_entry_count - 2; entry++) + for (int entry = 0; entry < fake_flash_entry_count - 1; entry++) memcpy(fake_flash_region + (entry * VB2_NVDATA_SIZE), test_nvdata_16b, sizeof(test_nvdata_16b)); memcpy(fake_flash_region + - ((fake_flash_entry_count - 2) * VB2_NVDATA_SIZE), + ((fake_flash_entry_count - 1) * VB2_NVDATA_SIZE), test_nvdata2_16b, sizeof(test_nvdata2_16b)); TEST_EQ(vb2_read_nv_storage_flashrom(&ctx), 0, @@ -222,7 +222,7 @@ static void test_write_ok_full(void) reset_test_data(&ctx, sizeof(test_nvdata_16b)); - for (int entry = 0; entry < fake_flash_entry_count - 1; entry++) + for (int entry = 0; entry < fake_flash_entry_count; entry++) memcpy(fake_flash_region + (entry * VB2_NVDATA_SIZE), test_nvdata_16b, sizeof(test_nvdata_16b)); |