Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36977 )
Change subject: cpu/x86/smm: Remove usused Kconfig symbol ......................................................................
cpu/x86/smm: Remove usused Kconfig symbol
These platforms now use parallel mp.
Change-Id: I73527a7feb6151d765295d6449262a2852b30177 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/x86/Kconfig M src/cpu/x86/smm/smmhandler.S 2 files changed, 0 insertions(+), 37 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/36977/1
diff --git a/src/cpu/x86/Kconfig b/src/cpu/x86/Kconfig index 85ebd83..5004cfd 100644 --- a/src/cpu/x86/Kconfig +++ b/src/cpu/x86/Kconfig @@ -137,13 +137,6 @@
endif
-config SMM_LAPIC_REMAP_MITIGATION - bool - default y if NORTHBRIDGE_INTEL_I945 - default y if NORTHBRIDGE_INTEL_GM45 - default y if NORTHBRIDGE_INTEL_NEHALEM - default n - config SERIALIZED_SMM_INITIALIZATION bool default n diff --git a/src/cpu/x86/smm/smmhandler.S b/src/cpu/x86/smm/smmhandler.S index a2be7f2..6e8941e 100644 --- a/src/cpu/x86/smm/smmhandler.S +++ b/src/cpu/x86/smm/smmhandler.S @@ -75,36 +75,6 @@ #endif .global smm_handler_start smm_handler_start: -#if CONFIG(SMM_LAPIC_REMAP_MITIGATION) - /* Check if the LAPIC register block overlaps with SMM. - * This block needs to work without data accesses because they - * may be routed into the LAPIC register block. - * Code accesses, on the other hand, are never routed to LAPIC, - * which is what makes this work in the first place. - */ - mov $LAPIC_BASE_MSR, %ecx - rdmsr - and $(~0xfff), %eax - sub $(SMM_START), %eax - cmp $(SMM_END - SMM_START), %eax - ja untampered_lapic -1: - /* emit "Crash" on serial */ - mov $(CONFIG_TTYS0_BASE), %dx - mov $'C', %al - out %al, (%dx) - mov $'r', %al - out %al, (%dx) - mov $'a', %al - out %al, (%dx) - mov $'s', %al - out %al, (%dx) - mov $'h', %al - out %al, (%dx) - /* now crash for real */ - ud2 -untampered_lapic: -#endif movw $(smm_gdtptr16 - smm_handler_start + SMM_HANDLER_OFFSET), %bx lgdtl %cs:(%bx)
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36977 )
Change subject: cpu/x86/smm: Remove usused Kconfig symbol ......................................................................
Patch Set 6: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/36977/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36977/6//COMMIT_MSG@7 PS6, Line 7: usused unused
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36977 )
Change subject: cpu/x86/smm: Remove usused Kconfig symbol ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36977/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36977/6//COMMIT_MSG@7 PS6, Line 7: usused
unused
Please identify the removed symbol on the title already.
Hello Angel Pons, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36977
to look at the new patch set (#7).
Change subject: cpu/x86/smm: Drop usused CONFIG_SMM_LAPIC_REMAP_MITIGATION symbol ......................................................................
cpu/x86/smm: Drop usused CONFIG_SMM_LAPIC_REMAP_MITIGATION symbol
These platforms now use parallel mp.
Change-Id: I73527a7feb6151d765295d6449262a2852b30177 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/x86/Kconfig M src/cpu/x86/smm/smmhandler.S 2 files changed, 0 insertions(+), 37 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/36977/7
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36977 )
Change subject: cpu/x86/smm: Drop usused CONFIG_SMM_LAPIC_REMAP_MITIGATION symbol ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36977/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36977/6//COMMIT_MSG@7 PS6, Line 7: usused
Please identify the removed symbol on the title already.
nit: usused still
Hello Angel Pons, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/36977
to look at the new patch set (#8).
Change subject: cpu/x86/smm: Drop unused CONFIG_SMM_LAPIC_REMAP_MITIGATION symbol ......................................................................
cpu/x86/smm: Drop unused CONFIG_SMM_LAPIC_REMAP_MITIGATION symbol
These platforms now use parallel mp.
Change-Id: I73527a7feb6151d765295d6449262a2852b30177 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/cpu/x86/Kconfig M src/cpu/x86/smm/smmhandler.S 2 files changed, 0 insertions(+), 37 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/36977/8
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36977 )
Change subject: cpu/x86/smm: Drop unused CONFIG_SMM_LAPIC_REMAP_MITIGATION symbol ......................................................................
Patch Set 8: Code-Review+2
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36977 )
Change subject: cpu/x86/smm: Drop unused CONFIG_SMM_LAPIC_REMAP_MITIGATION symbol ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36977/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/36977/6//COMMIT_MSG@7 PS6, Line 7: usused
nit: usused still
Done
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36977 )
Change subject: cpu/x86/smm: Drop unused CONFIG_SMM_LAPIC_REMAP_MITIGATION symbol ......................................................................
Patch Set 8:
I can't seem to find this mitigation in the parallel mp path, so I guess we're open to the Memory Sinkhole issue again (just that we didn't notice because the code was still here)?
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36977 )
Change subject: cpu/x86/smm: Drop unused CONFIG_SMM_LAPIC_REMAP_MITIGATION symbol ......................................................................
Patch Set 8:
Patch Set 8:
I can't seem to find this mitigation in the parallel mp path, so I guess we're open to the Memory Sinkhole issue again (just that we didn't notice because the code was still here)?
I'm pretty confused by this code. Why would the lapic base be programmed to be at or close to SMMBASE? There are a lot of ranges that SMM should not conflict with...
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36977 )
Change subject: cpu/x86/smm: Drop unused CONFIG_SMM_LAPIC_REMAP_MITIGATION symbol ......................................................................
Patch Set 8:
Patch Set 8:
Patch Set 8:
I can't seem to find this mitigation in the parallel mp path, so I guess we're open to the Memory Sinkhole issue again (just that we didn't notice because the code was still here)?
I'm pretty confused by this code. Why would the lapic base be programmed to be at or close to SMMBASE? There are a lot of ranges that SMM should not conflict with...
It's mitigation for an attack: by carefully choosing your lapic base, you can inject data into ASEG (or TSEG, but this code doesn't protect against that) on the i945-nehalem chipsets.
When knowing the SMM code flow (and the resulting data access pattern) it may be possible to put the lapic just right so that the first few accesses from within SMM hit well-known LAPIC bytes instead of the data that is supposed to be read. This can change the behavior of the code in ways favorable to an attacker. https://github.com/xoreaxeaxeax/sinkhole has a proof of concept.
Since LAPIC is indeed never supposed to be configured to overlap SMM under normal circumstances, this code is made to crash in such a situation.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36977 )
Change subject: cpu/x86/smm: Drop unused CONFIG_SMM_LAPIC_REMAP_MITIGATION symbol ......................................................................
Patch Set 8:
Patch Set 8:
Patch Set 8:
Patch Set 8:
I can't seem to find this mitigation in the parallel mp path, so I guess we're open to the Memory Sinkhole issue again (just that we didn't notice because the code was still here)?
I'm pretty confused by this code. Why would the lapic base be programmed to be at or close to SMMBASE? There are a lot of ranges that SMM should not conflict with...
It's mitigation for an attack: by carefully choosing your lapic base, you can inject data into ASEG (or TSEG, but this code doesn't protect against that) on the i945-nehalem chipsets.
When knowing the SMM code flow (and the resulting data access pattern) it may be possible to put the lapic just right so that the first few accesses from within SMM hit well-known LAPIC bytes instead of the data that is supposed to be read. This can change the behavior of the code in ways favorable to an attacker. https://github.com/xoreaxeaxeax/sinkhole has a proof of concept.
Oh wow, that's bad!
Since LAPIC is indeed never supposed to be configured to overlap SMM under normal circumstances, this code is made to crash in such a situation.
Sounds like a good protection mechanism. Any reason this is only needed on i945-nehalem?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36977 )
Change subject: cpu/x86/smm: Drop unused CONFIG_SMM_LAPIC_REMAP_MITIGATION symbol ......................................................................
Patch Set 8:
Sounds like a good protection mechanism. Any reason this is only needed on i945-nehalem?
On Intel: Sandybridge (the next newer one we supported back then) does that check itself in "hardware" (with the assumption that following chipsets will do as well) while for pre-i945 we didn't have SMM.
AMD/Via: Stuff is sufficiently different that they don't fail the same way.
non-x86: no SMM \o/
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36977 )
Change subject: cpu/x86/smm: Drop unused CONFIG_SMM_LAPIC_REMAP_MITIGATION symbol ......................................................................
Patch Set 8: Code-Review-2
Attempt to port this to the relocatable stub: CB:37289 (untested)
Kyösti Mälkki has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/36977 )
Change subject: cpu/x86/smm: Drop unused CONFIG_SMM_LAPIC_REMAP_MITIGATION symbol ......................................................................
Removed Code-Review+2 by Kyösti Mälkki kyosti.malkki@gmail.com
Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/36977?usp=email )
Change subject: cpu/x86/smm: Drop unused CONFIG_SMM_LAPIC_REMAP_MITIGATION symbol ......................................................................
Abandoned
This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Kyösti Mälkki (Code Review)
-
Martin L Roth (Code Review)
-
Patrick Georgi (Code Review)