[coreboot-gerrit] Change in coreboot[master]: cbfs: Move more stuff into cbfs_boot_lookup()

[Julius Werner (Code Review)]

Hello Aaron Durbin,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/39327

to review the following change.

Change subject: cbfs: Move more stuff into cbfs_boot_lookup()
......................................................................

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().

Signed-off-by: Julius Werner 
Change-Id: I4bc9b7cbc42a4211d806a3e3389abab7f589a25a
---
M src/lib/cbfs.c
M src/security/vboot/vboot_crtm.c
M src/security/vboot/vboot_crtm.h
3 files changed, 34 insertions(+), 32 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/39327/1

diff --git a/src/lib/cbfs.c b/src/lib/cbfs.c
index 39db97a..ef29299 100644
--- a/src/lib/cbfs.c
+++ b/src/lib/cbfs.c
@@ -31,40 +31,47 @@
 #include <security/vboot/vboot_crtm.h>
 #include <security/vboot/vboot_common.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)
+static cb_err_t cbfs_boot_lookup(struct region_device *rdev, const char *name,
+		union cbfs_mdata *mdata, size_t *data_offset, bool force_ro)
 {
+	cb_err_t err;
+	const struct cbfs_boot_device *cbd = cbfs_get_boot_device(force_ro);
+	if (!cbd || rdev_chain_full(rdev, &cbd->rdev))
+		return CB_ERR;
+
    if (CONFIG(NO_CBFS_MCACHE))
-		return cbfs_lookup(&cbd->rdev, name, mdata, data_offset, NULL);
+		err = cbfs_lookup(rdev, name, mdata, data_offset, NULL);
    else
-		return cbfs_mcache_lookup(&cbd->rdev,
-					  cbd->mcache, cbd->mcache_size,
-					  name, mdata, data_offset, NULL);
+		err = cbfs_mcache_lookup(rdev, cbd->mcache, cbd->mcache_size,
+					 name, mdata, data_offset, NULL);
+
+	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);
+		return cbfs_boot_lookup(rdev, name, mdata, data_offset, true);
+	}
+	if (err)
+		return err;
+
+	if (vboot_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)
 {
-	const struct cbfs_boot_device *cbd = cbfs_get_boot_device(false);
-	if (!cbd)
-		return -1;
-
+	struct region_device rdev;
    union cbfs_mdata mdata;
    size_t data_offset;
-	cb_err_t err = cbfs_boot_lookup(cbd, name, &mdata, &data_offset);

-	if (CONFIG(VBOOT_ENABLE_CBFS_FALLBACK) && 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, &mdata, &data_offset);
-	}
-	if (err)
+	if (cbfs_boot_lookup(&rdev, name, &mdata, &data_offset, false))
    	return -1;

size_t msize = be32toh(mdata.h.offset);
-	if (rdev_chain(&fh->metadata, &cbd->rdev, data_offset - msize, msize) ||
-	    rdev_chain(&fh->data, &cbd->rdev, data_offset, be32toh(mdata.h.len)))
+	if (rdev_chain(&fh->metadata, &rdev, data_offset - msize, msize) ||
+	    rdev_chain(&fh->data, &rdev, data_offset, be32toh(mdata.h.len)))
    	return -1;
    if (type) {
    	if (!*type)
@@ -73,9 +80,6 @@
    		return -1;
    }

-	if (vboot_measure_cbfs_hook(fh, name))
-		return -1;
-
    return 0;
 }

diff --git a/src/security/vboot/vboot_crtm.c b/src/security/vboot/vboot_crtm.c
index f68ab0a4..75c5662 100644
--- a/src/security/vboot/vboot_crtm.c
+++ b/src/security/vboot/vboot_crtm.c
@@ -157,19 +157,16 @@
    return false;
 }

-uint32_t vboot_measure_cbfs_hook(struct cbfsf *fh, const char *name)
+uint32_t vboot_measure_cbfs_hook(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 (!vboot_logic_executed())
    	return 0;

-	cbfsf_file_type(fh, &cbfs_type);
-	cbfs_file_data(&rdev, fh);
-
    switch (cbfs_type) {
    case CBFS_TYPE_MRC:
    case CBFS_TYPE_MRC_CACHE:
diff --git a/src/security/vboot/vboot_crtm.h b/src/security/vboot/vboot_crtm.h
index 64cb4f2..8e6c076 100644
--- a/src/security/vboot/vboot_crtm.h
+++ b/src/security/vboot/vboot_crtm.h
@@ -49,13 +49,14 @@
 #if CONFIG(VBOOT_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 vboot_measure_cbfs_hook(struct cbfsf *fh, const char *name);
+uint32_t vboot_measure_cbfs_hook(struct region_device *rdev, const char *name,
+				 uint32_t cbfs_type);

#else
-#define vboot_measure_cbfs_hook(fh, name) 0
+#define vboot_measure_cbfs_hook(rdev, name, cbfs_type) 0
 #endif

#endif /* __VBOOT_VBOOT_CRTM_H__ */