Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47069 )
Change subject: cpu/x86/smm: Check that the stub size is < save state size ......................................................................
cpu/x86/smm: Check that the stub size is < save state size
If the stub size would be larger than the save state size. The check is placed in the stub placement code. The stub placement code is called twice. Once for the initial SMM relocatation and for the permanent handler in TSEG. So the check is done twice, which is not really needed.
Change-Id: I253e1a7112cd8f7496cb1a826311f4dd5ccfc73a Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/x86/smm/smm_module_loader.c M src/cpu/x86/smm/smm_module_loaderv2.c 2 files changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/47069/1
diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index fc1e1b3..876fde6 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -209,6 +209,12 @@ smm_stub_size = rmodule_memory_size(&smm_stub); stub_entry_offset = rmodule_entry_offset(&smm_stub);
+ if (smm_stub_size > params->per_cpu_save_state_size) { + printk(BIOS_ERR, "SMM Module: SMM stub size larger than save state size\n"); + printk(BIOS_ERR, "SMM Module: Staggered entry points will overlap stub\n"); + return -1; + } + /* Assume the stub is always small enough to live within upper half of * SMRAM region after the save state space has been allocated. */ smm_stub_loc = &base[SMM_ENTRY_OFFSET]; diff --git a/src/cpu/x86/smm/smm_module_loaderv2.c b/src/cpu/x86/smm/smm_module_loaderv2.c index 22f336e..3fa5871 100644 --- a/src/cpu/x86/smm/smm_module_loaderv2.c +++ b/src/cpu/x86/smm/smm_module_loaderv2.c @@ -134,6 +134,12 @@ return 0; }
+ if (stub_size > ss_size) { + printk(BIOS_ERR, "%s: Save state larger than SMM stub size\n", __func__); + printk(BIOS_ERR, " Decrease stub size or increase the size allocated for the save state\n"); + return 0; + } + for (i = 0; i < num_cpus; i++) { cpus[i].smbase = base; cpus[i].entry = base + smm_entry_offset;
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47069 )
Change subject: cpu/x86/smm: Check that the stub size is < save state size ......................................................................
Patch Set 1: Code-Review+1
Hello build bot (Jenkins), Angel Pons, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/47069
to look at the new patch set (#2).
Change subject: cpu/x86/smm: Check that the stub size is < save state size ......................................................................
cpu/x86/smm: Check that the stub size is < save state size
If the stub size would be larger than the save state size, the stagger points would overlap with the stub.
The check is placed in the stub placement code. The stub placement code is called twice. Once for the initial SMM relocatation and for the permanent handler in TSEG. So the check is done twice, which is not really needed.
Change-Id: I253e1a7112cd8f7496cb1a826311f4dd5ccfc73a Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/x86/smm/smm_module_loader.c M src/cpu/x86/smm/smm_module_loaderv2.c 2 files changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/47069/2
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47069 )
Change subject: cpu/x86/smm: Check that the stub size is < save state size ......................................................................
Patch Set 2: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47069 )
Change subject: cpu/x86/smm: Check that the stub size is < save state size ......................................................................
Patch Set 2: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47069 )
Change subject: cpu/x86/smm: Check that the stub size is < save state size ......................................................................
cpu/x86/smm: Check that the stub size is < save state size
If the stub size would be larger than the save state size, the stagger points would overlap with the stub.
The check is placed in the stub placement code. The stub placement code is called twice. Once for the initial SMM relocatation and for the permanent handler in TSEG. So the check is done twice, which is not really needed.
Change-Id: I253e1a7112cd8f7496cb1a826311f4dd5ccfc73a Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/47069 Reviewed-by: David Hendricks david.hendricks@gmail.com Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/cpu/x86/smm/smm_module_loader.c M src/cpu/x86/smm/smm_module_loaderv2.c 2 files changed, 12 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified David Hendricks: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/cpu/x86/smm/smm_module_loader.c b/src/cpu/x86/smm/smm_module_loader.c index fc1e1b3..876fde6 100644 --- a/src/cpu/x86/smm/smm_module_loader.c +++ b/src/cpu/x86/smm/smm_module_loader.c @@ -209,6 +209,12 @@ smm_stub_size = rmodule_memory_size(&smm_stub); stub_entry_offset = rmodule_entry_offset(&smm_stub);
+ if (smm_stub_size > params->per_cpu_save_state_size) { + printk(BIOS_ERR, "SMM Module: SMM stub size larger than save state size\n"); + printk(BIOS_ERR, "SMM Module: Staggered entry points will overlap stub\n"); + return -1; + } + /* Assume the stub is always small enough to live within upper half of * SMRAM region after the save state space has been allocated. */ smm_stub_loc = &base[SMM_ENTRY_OFFSET]; diff --git a/src/cpu/x86/smm/smm_module_loaderv2.c b/src/cpu/x86/smm/smm_module_loaderv2.c index 22f336e..3fa5871 100644 --- a/src/cpu/x86/smm/smm_module_loaderv2.c +++ b/src/cpu/x86/smm/smm_module_loaderv2.c @@ -134,6 +134,12 @@ return 0; }
+ if (stub_size > ss_size) { + printk(BIOS_ERR, "%s: Save state larger than SMM stub size\n", __func__); + printk(BIOS_ERR, " Decrease stub size or increase the size allocated for the save state\n"); + return 0; + } + for (i = 0; i < num_cpus; i++) { cpus[i].smbase = base; cpus[i].entry = base + smm_entry_offset;
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
David Hendricks (Code Review)
-
Patrick Georgi (Code Review)