aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFelix Held <felix-coreboot@felixheld.de>2021-02-02 21:30:25 +0100
committerFelix Held <felix-coreboot@felixheld.de>2021-02-23 01:32:14 +0000
commita09559501c06e3cab420908bd667af4486ec0ba5 (patch)
tree1f9ff2e768cddcf41a519ec2e82e37c7a98b7bfc
parentca5ed4879fd859b050682591459113f39d4caf84 (diff)
drivers/intel/fsp2_0/memory_init: check if UPD struct has expected size
If the UPD size in coreboot sizes mismatches the one from the FSP-M binary, we're running into trouble. If the expected size is smaller than the UPD size the FSP provides, call die(), since the target buffer isn't large enough so only the beginning of the UPD defaults from the FSP will get copied into the buffer. We ran into the issue in soc/amd/cezanne, where the UPD struct in coreboot was smaller than the one in the FSP, so the defaults didn't get completely copied. TEST=Mandolin still boots. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d Reviewed-on: https://review.coreboot.org/c/coreboot/+/50241 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
-rw-r--r--src/drivers/intel/fsp2_0/memory_init.c17
1 files changed, 17 insertions, 0 deletions
diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c
index f2fcec4061..0ea5c08c43 100644
--- a/src/drivers/intel/fsp2_0/memory_init.c
+++ b/src/drivers/intel/fsp2_0/memory_init.c
@@ -239,6 +239,23 @@ static void do_fsp_memory_init(const struct fspm_context *context, bool s3wake)
upd = (FSPM_UPD *)(uintptr_t)(hdr->cfg_region_offset + hdr->image_base);
+ /*
+ * Verify UPD region size. We don't have malloc before ramstage, so we
+ * use a static buffer for the FSP-M UPDs which is sizeof(FSPM_UPD)
+ * bytes long, since that is the value known at compile time. If
+ * hdr->cfg_region_size is bigger than that, not all UPD defaults will
+ * be copied, so it'll contain random data at the end, so we just call
+ * die() in that case. If hdr->cfg_region_size is smaller than that,
+ * there's a mismatch between the FSP and the header, but since it will
+ * copy the full UPD defaults to the buffer, we try to continue and
+ * hope that there was no incompatible change in the UPDs.
+ */
+ if (hdr->cfg_region_size > sizeof(FSPM_UPD))
+ die("FSP-M UPD size is larger than FSPM_UPD struct size.\n");
+ if (hdr->cfg_region_size < sizeof(FSPM_UPD))
+ printk(BIOS_ERR, "FSP-M UPD size is smaller than FSPM_UPD struct size. "
+ "Check if the FSP binary matches the FSP headers.\n");
+
fsp_verify_upd_header_signature(upd->FspUpdHeader.Signature, FSPM_UPD_SIGNATURE);
/* Copy the default values from the UPD area */