Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35349 )
Change subject: drivers/intel/fsp2_0: Allocate cfg_region_size for UPD ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35349/3/src/drivers/intel/fsp2_0/si... File src/drivers/intel/fsp2_0/silicon_init.c:
https://review.coreboot.org/c/coreboot/+/35349/3/src/drivers/intel/fsp2_0/si... PS3, Line 32: FSPS_UPD *upd = NULL; This change can be dropped.
https://review.coreboot.org/c/coreboot/+/35349/3/src/drivers/intel/fsp2_0/si... PS3, Line 47: < != ??
There's some assumed behavior here that actually doesn't play out in practice for people maintaining a FSP:
1. new UPDs are added to the end preserving previous behavior of the existing fields. 2. there's no reshuffling of fields
Both are not true in practice from my experience. That's why I think we should check with !=. The memcpy() below will overlow the allocation if cfg_region_size exceeds the FSPS_UPD object. That's no good.