From a09559501c06e3cab420908bd667af4486ec0ba5 Mon Sep 17 00:00:00 2001 From: Felix Held Date: Tue, 2 Feb 2021 21:30:25 +0100 Subject: 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 Change-Id: Ia7e9f6f20d0091bbb4abfd42abb40b485da2079d Reviewed-on: https://review.coreboot.org/c/coreboot/+/50241 Tested-by: build bot (Jenkins) Reviewed-by: Tim Wawrzynczak --- src/drivers/intel/fsp2_0/memory_init.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) 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 */ -- cgit v1.2.3