It looks like this bug is biting us now.
https://review.coreboot.org/c/coreboot/+/64521 removed the heap from SMM (because it's not needed and a bad idea).
Now that the heap is gone the FX_SAVE area is actually overwriting the handler. So this vulnerability is not hypothetical anymore
it breaks the smihandler on all SSE platforms.

Please review the fix in https://review.coreboot.org/c/coreboot/+/63475 or alternatively https://review.coreboot.org/c/coreboot/+/63560 (rewrite using regions to check for overlaps at runtime).

Kind regards,

On Tue, Apr 12, 2022 at 10:46 AM Arthur Heymans <arthur@aheymans.xyz> wrote:

The obvious easy solution is to not use SMM but that's a different topic.

I think it should also be doable to write unit tests that would do a setup for 1 (1 is often a special case) and many cpus and see that things like stubs, stack, save state, permanent handler, ... don't overlap.
I played around with our unit test framework and I'm impressed. We should use it way more ;-)
https://review.coreboot.org/c/coreboot/+/63560 is where I did some very very WIP unit test(s) on SMM loading and I'm already able to load specially crafted relocatable modules for both stubs and permanent handler.
I think it should be possible to test all kinds of good and bad configurations and make this code more robust and future proof.

Kind regards

On Mon, Apr 11, 2022 at 6:11 PM ron minnich <rminnich@gmail.com> wrote:
arthur, what might we do with either the build process or startup to
avoid this problem in future? Do you think we could find a way to
catch this programmatically soon, rather than humanly too late?

On Mon, Apr 11, 2022 at 2:48 AM Arthur Heymans <arthur@aheymans.xyz> wrote:
> Hi
> After last week's SMM loader problem on all but the BSP, I noticed another problem in the SMM setup.
> The permanent smihandler is currently built as a relocatable module such that coreboot
> can place it wherever it thinks it's a good idea. (TSEG is not known at buildtime).
> These relocatable modules have an alignment requirement.
> It looks however that the code to deal with the alignment requirement is also wrong
> and aligns the handler upwards instead of downwards which makes it encroach either an SSE2
> FX_SAVE area or an SMM register save state. It's hard to know whether this is easily exploitable.
> I would think that a carefully crafted SMM save state on the right AP arbitrary code executing might be possible. On the other hand I noticed last week that launching SMM on APs is broken too so this is likely a lesser problem.
> Anyway the fix is in https://review.coreboot.org/c/coreboot/+/63475
> (It has a comment indicating what code was causing this problem)
> Please review and update your coreboot code!
> Kind regards
> Arthur
> _______________________________________________
> coreboot mailing list -- coreboot@coreboot.org
> To unsubscribe send an email to coreboot-leave@coreboot.org