diff options
author | Felix Held <felix-coreboot@felixheld.de> | 2021-02-02 21:30:25 +0100 |
---|---|---|
committer | Felix Held <felix-coreboot@felixheld.de> | 2021-02-23 01:32:14 +0000 |
commit | a09559501c06e3cab420908bd667af4486ec0ba5 (patch) | |
tree | 1f9ff2e768cddcf41a519ec2e82e37c7a98b7bfc | |
parent | ca5ed4879fd859b050682591459113f39d4caf84 (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.c | 17 |
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 */ |