cbfs: Move more stuff into cbfs_boot_lookup()

cbfs_boot_locate() is supposed to be deprecated eventually, after slowly
migrating all APIs to bypass it. That means common features (like
RO-fallback or measurement) need to be moved to the new
cbfs_boot_lookup().

Also export the function externally. Since it is a low-level API and
most code should use the higher-level loading or mapping functions
instead, put it into a new <cbfs_private.h> to raise the mental barrier
for using this API (this will make more sense once cbfs_boot_locate() is
removed from <cbfs.h>).

Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I4bc9b7cbc42a4211d806a3e3389abab7f589a25a
Reviewed-on: https://review.coreboot.org/c/coreboot/+/39327
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>

6 files changed, 73 insertions, 40 deletions

diff --git a/src/include/boot_device.h b/src/include/boot_device.h
index a03e5aa8f3..84bd16ef65 100644
--- a/src/include/boot_device.h
+++ b/src/include/boot_device.h
@@ -27,7 +27,8 @@ enum bootdev_prot_type {
  * most likely not to work so don't rely on such semantics.
  */
 
-/* Return the region_device for the read-only boot device. */
+/* Return the region_device for the read-only boot device. This is the root
+   device for all CBFS boot devices. */
 const struct region_device *boot_device_ro(void);
 
 /* Return the region_device for the read-write boot device. */
diff --git a/src/include/cbfs.h b/src/include/cbfs.h
index 32ed7f899e..1b446ac7a8 100644
--- a/src/include/cbfs.h
+++ b/src/include/cbfs.h
@@ -42,6 +42,12 @@ size_t cbfs_load_and_decompress(const struct region_device *rdev, size_t offset,
 /* Load stage into memory filling in prog. Return 0 on success. < 0 on error. */
 int cbfs_prog_stage_load(struct prog *prog);
 
+/*
+ * Data structure that represents "a" CBFS boot device, with optional metadata
+ * cache. Generally we only have one of these, or two (RO and RW) when
+ * CONFIG(VBOOT) is set. The region device stored here must always be a
+ * subregion of boot_device_ro().
+ */
 struct cbfs_boot_device {
 	struct region_device rdev;
 	void *mcache;
diff --git a/src/include/cbfs_private.h b/src/include/cbfs_private.h
new file mode 100644
index 0000000000..8e9803616f
--- /dev/null
+++ b/src/include/cbfs_private.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _CBFS_PRIVATE_H_
+#define _CBFS_PRIVATE_H_
+
+#include <commonlib/bsd/cbfs_private.h>
+#include <commonlib/region.h>
+#include <types.h>
+
+/*
+ * This header contains low-level CBFS APIs that should only be used by code
+ * that really needs this level of access. Most code (particularly platform
+ * code) should use the higher-level CBFS APIs in <cbfs.h>. Code using these
+ * APIs needs to take special care to ensure CBFS file data is verified (in a
+ * TOCTOU-safe manner) before access (TODO: add details on how to do this once
+ * file verification code is in).
+ */
+
+/* Find by name, load metadata into |mdata| and chain file data to |rdev|. */
+cb_err_t cbfs_boot_lookup(const char *name, bool force_ro,
+			  union cbfs_mdata *mdata, struct region_device *rdev);
+
+#endif
diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c
index 3b7d4292d0..94dae625b2 100644
--- a/src/lib/cbfs.c
+++ b/src/lib/cbfs.c
@@ -3,8 +3,8 @@
 #include <assert.h>
 #include <boot_device.h>
 #include <cbfs.h>
+#include <cbfs_private.h>
 #include <cbmem.h>
-#include <commonlib/bsd/cbfs_private.h>
 #include <commonlib/bsd/compression.h>
 #include <commonlib/endian.h>
 #include <console/console.h>
@@ -17,43 +17,49 @@
 #include <symbols.h>
 #include <timestamp.h>
 
-static cb_err_t cbfs_boot_lookup(const struct cbfs_boot_device *cbd,
-		const char *name, union cbfs_mdata *mdata, size_t *data_offset)
+cb_err_t cbfs_boot_lookup(const char *name, bool force_ro,
+			  union cbfs_mdata *mdata, struct region_device *rdev)
 {
+	const struct cbfs_boot_device *cbd = cbfs_get_boot_device(force_ro);
+	if (!cbd)
+		return CB_ERR;
+
+	size_t data_offset;
 	cb_err_t err = CB_CBFS_CACHE_FULL;
 	if (!CONFIG(NO_CBFS_MCACHE) && !ENV_SMM)
 		err = cbfs_mcache_lookup(cbd->mcache, cbd->mcache_size,
-					  name, mdata, data_offset);
+					  name, mdata, &data_offset);
 	if (err == CB_CBFS_CACHE_FULL)
-		err = cbfs_lookup(&cbd->rdev, name, mdata, data_offset, NULL);
-	return err;
-}
-
-int cbfs_boot_locate(struct cbfsf *fh, const char *name, uint32_t *type)
-{
-	const struct cbfs_boot_device *cbd = cbfs_get_boot_device(false);
-	if (!cbd)
-		return -1;
+		err = cbfs_lookup(&cbd->rdev, name, mdata, &data_offset, NULL);
 
-	size_t data_offset;
-	cb_err_t err = cbfs_boot_lookup(cbd, name, &fh->mdata, &data_offset);
-
-	if (CONFIG(VBOOT_ENABLE_CBFS_FALLBACK) && err == CB_CBFS_NOT_FOUND) {
+	if (CONFIG(VBOOT_ENABLE_CBFS_FALLBACK) && !force_ro &&
+	    err == CB_CBFS_NOT_FOUND) {
 		printk(BIOS_INFO, "CBFS: Fall back to RO region for %s\n",
 		       name);
-		if (!(cbd = cbfs_get_boot_device(true)))
-			return -1;
-		err = cbfs_boot_lookup(cbd, name, &fh->mdata, &data_offset);
+		return cbfs_boot_lookup(name, true, mdata, rdev);
 	}
 	if (err)
+		return err;
+
+	if (rdev_chain(rdev, &cbd->rdev, data_offset, be32toh(mdata->h.len)))
+		return CB_ERR;
+
+	if (tspi_measure_cbfs_hook(rdev, name, be32toh(mdata->h.type)))
+		return CB_ERR;
+
+	return CB_SUCCESS;
+}
+
+int cbfs_boot_locate(struct cbfsf *fh, const char *name, uint32_t *type)
+{
+	if (cbfs_boot_lookup(name, false, &fh->mdata, &fh->data))
 		return -1;
 
 	size_t msize = be32toh(fh->mdata.h.offset);
 	if (rdev_chain(&fh->metadata, &addrspace_32bit.rdev,
-		       (uintptr_t)&fh->mdata, msize) ||
-	    rdev_chain(&fh->data, &cbd->rdev, data_offset,
-		       be32toh(fh->mdata.h.len)))
+		       (uintptr_t)&fh->mdata, msize))
 		return -1;
+
 	if (type) {
 		if (!*type)
 			*type = be32toh(fh->mdata.h.type);
@@ -61,9 +67,6 @@ int cbfs_boot_locate(struct cbfsf *fh, const char *name, uint32_t *type)
 			return -1;
 	}
 
-	if (tspi_measure_cbfs_hook(fh, name))
-		return -1;
-
 	return 0;
 }
 
@@ -94,9 +97,13 @@ int cbfs_locate_file_in_region(struct cbfsf *fh, const char *region_name,
 		return -1;
 	}
 
+	uint32_t dummy_type = 0;
+	if (!type)
+		type = &dummy_type;
+
 	ret = cbfs_locate(fh, &rdev, name, type);
 	if (!ret)
-		if (tspi_measure_cbfs_hook(fh, name))
+		if (tspi_measure_cbfs_hook(&rdev, name, *type))
 			return -1;
 	return ret;
 }
diff --git a/src/security/tpm/tspi/crtm.c b/src/security/tpm/tspi/crtm.c
index eb0744209c..80483d575e 100644
--- a/src/security/tpm/tspi/crtm.c
+++ b/src/security/tpm/tspi/crtm.c
@@ -102,11 +102,10 @@ static bool is_runtime_data(const char *name)
 	return !strcmp(allowlist, name);
 }
 
-uint32_t tspi_measure_cbfs_hook(struct cbfsf *fh, const char *name)
+uint32_t tspi_measure_cbfs_hook(const struct region_device *rdev, const char *name,
+				uint32_t cbfs_type)
 {
 	uint32_t pcr_index;
-	uint32_t cbfs_type;
-	struct region_device rdev;
 	char tcpa_metadata[TCPA_PCR_HASH_NAME];
 
 	if (!tcpa_log_available()) {
@@ -118,9 +117,6 @@ uint32_t tspi_measure_cbfs_hook(struct cbfsf *fh, const char *name)
 		printk(BIOS_DEBUG, "CRTM initialized.\n");
 	}
 
-	cbfsf_file_type(fh, &cbfs_type);
-	cbfs_file_data(&rdev, fh);
-
 	switch (cbfs_type) {
 	case CBFS_TYPE_MRC_CACHE:
 		pcr_index = TPM_RUNTIME_DATA_PCR;
@@ -143,10 +139,10 @@ uint32_t tspi_measure_cbfs_hook(struct cbfsf *fh, const char *name)
 		break;
 	}
 
-	if (create_tcpa_metadata(&rdev, name, tcpa_metadata) < 0)
+	if (create_tcpa_metadata(rdev, name, tcpa_metadata) < 0)
 		return VB2_ERROR_UNKNOWN;
 
-	return tpm_measure_region(&rdev, pcr_index, tcpa_metadata);
+	return tpm_measure_region(rdev, pcr_index, tcpa_metadata);
 }
 
 int tspi_measure_cache_to_pcr(void)
diff --git a/src/security/tpm/tspi/crtm.h b/src/security/tpm/tspi/crtm.h
index 1b29854efb..f3678ef033 100644
--- a/src/security/tpm/tspi/crtm.h
+++ b/src/security/tpm/tspi/crtm.h
@@ -41,13 +41,13 @@ int tspi_measure_cache_to_pcr(void);
 #if !ENV_SMM && CONFIG(TPM_MEASURED_BOOT)
 /*
  * Measures cbfs data via hook (cbfs)
- * fh is the cbfs file handle to measure
+ * rdev covers the file data (not metadata)
  * return 0 if successful, else an error
  */
-uint32_t tspi_measure_cbfs_hook(struct cbfsf *fh, const char *name);
-
+uint32_t tspi_measure_cbfs_hook(const struct region_device *rdev,
+				const char *name, uint32_t cbfs_type);
 #else
-#define tspi_measure_cbfs_hook(fh, name) 0
+#define tspi_measure_cbfs_hook(rdev, name, cbfs_type) 0
 #endif
 
 #endif /* __SECURITY_TSPI_CRTM_H__ */