Julius Werner submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve Christian Walter: Looks good to me, approved
cbfs: Allow mcache to be found after the first lookup

This patch addresses the same problem as CB:48429, but hopefully this
time correctly. Since the mcache is not guaranteed to be available on
the first CBFS lookup for some special cases, we can no longer treat it
as a one-time fire-and-forget initialization. Instead, we test
cbd->mcache_size to check if the mcache has been initialized yet, and
keep trying on every lookup if we don't find it the first time.

Since the mcache is a hard requirement for TOCTOU safety, also make it
more clear in Kconfig that configurations known to do CBFS accesses
before CBMEM init are incompatbile with that, and make sure we die()
rather than do something unsafe if there's a case that Kconfig didn't
catch.

Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I4e01e9a9905f7dcba14eaf05168495201ed5de60
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48482
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Christian Walter <christian.walter@9elements.com>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
---
M src/lib/Kconfig.cbfs_verification
M src/lib/cbfs.c
2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/lib/Kconfig.cbfs_verification b/src/lib/Kconfig.cbfs_verification
index 3499345..4e2ed8c 100644
--- a/src/lib/Kconfig.cbfs_verification
+++ b/src/lib/Kconfig.cbfs_verification
@@ -19,6 +19,7 @@
depends on CBFS_VERIFICATION
depends on !NO_FMAP_CACHE
depends on !NO_CBFS_MCACHE
+ depends on !USE_OPTION_TABLE && !FSP_CAR # Known to access CBFS before CBMEM init
help
Work in progress. Not actually TOCTOU safe yet. Do not use.

diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c
index d275505..60dc15a 100644
--- a/src/lib/cbfs.c
+++ b/src/lib/cbfs.c
@@ -27,7 +27,7 @@

size_t data_offset;
cb_err_t err = CB_CBFS_CACHE_FULL;
- if (!CONFIG(NO_CBFS_MCACHE) && !ENV_SMM)
+ if (!CONFIG(NO_CBFS_MCACHE) && !ENV_SMM && cbd->mcache_size)
err = cbfs_mcache_lookup(cbd->mcache, cbd->mcache_size,
name, mdata, &data_offset);
if (err == CB_CBFS_CACHE_FULL) {
@@ -35,6 +35,8 @@
if (CONFIG(TOCTOU_SAFETY)) {
if (ENV_SMM) /* Cannot provide TOCTOU safety for SMM */
dead_code();
+ if (!cbd->mcache_size)
+ die("Cannot access CBFS TOCTOU-safely in " ENV_STRING " before CBMEM init!\n");
/* We can only reach this for the RW CBFS -- an mcache
overflow in the RO CBFS would have been caught when
building the mcache in cbfs_get_boot_device().
@@ -408,6 +410,9 @@
if (CONFIG(NO_CBFS_MCACHE) || ENV_SMM)
return;

+ if (cbd->mcache_size)
+ return;
+
const struct cbmem_entry *entry;
if (cbmem_possibly_online() &&
(entry = cbmem_entry_find(id))) {
@@ -465,14 +470,17 @@
return rw;
}

+ /* In rare cases post-RAM stages may run this before cbmem_initialize(),
+ so we can't lock in the result of find_mcache() on the first try and
+ should keep trying every time until an mcache is found. */
+ cbfs_boot_device_find_mcache(&ro, CBMEM_ID_CBFS_RO_MCACHE);
+
if (region_device_sz(&ro.rdev))
return &ro;

if (fmap_locate_area_as_rdev("COREBOOT", &ro.rdev))
die("Cannot locate primary CBFS");

- cbfs_boot_device_find_mcache(&ro, CBMEM_ID_CBFS_RO_MCACHE);
-
if (ENV_INITIAL_STAGE) {
cb_err_t err = cbfs_init_boot_device(&ro, metadata_hash_get());
if (err == CB_CBFS_HASH_MISMATCH)

To view, visit change 48482. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4e01e9a9905f7dcba14eaf05168495201ed5de60
Gerrit-Change-Number: 48482
Gerrit-PatchSet: 4
Gerrit-Owner: Julius Werner <jwerner@chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz>
Gerrit-Reviewer: Christian Walter <christian.walter@9elements.com>
Gerrit-Reviewer: Furquan Shaikh <furquan@google.com>
Gerrit-Reviewer: Julius Werner <jwerner@chromium.org>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-MessageType: merged