Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43976 )
Change subject: lib/Makefile.inc: Enable UBSAN on SMM, too ......................................................................
lib/Makefile.inc: Enable UBSAN on SMM, too
We don't want undefined behavior on such privileged code, do we?
Change-Id: I24f4c074ff90365cf96abf1f5bad3e2cde68547d Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/lib/Makefile.inc 1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/43976/1
diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc index 72d4f24..c96aa90 100644 --- a/src/lib/Makefile.inc +++ b/src/lib/Makefile.inc @@ -4,7 +4,9 @@
ifeq ($(CONFIG_UBSAN),y) ramstage-y += ubsan.c +smm-y += ubsan.c CFLAGS_ramstage += -fsanitize=undefined +CFLAGS_smm += -fsanitize=undefined endif
decompressor-y += decompressor.c
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43976 )
Change subject: lib/Makefile.inc: Enable UBSAN on SMM, too ......................................................................
Patch Set 1:
We don't want SMM running into UB, but I'm not sure we can easily enable this here: The sanitizers usually have some extra memory somewhere to keep track of things, and while I'm not sure if that's true for UBSAN, if it is, we ought to ensure that this memory isn't somewhere in userspace (both due to the resulting attack surface against SMM and because SMM could unwittingly corrupt non-SMM data if that stuff is simply placed anywhere)
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43976 )
Change subject: lib/Makefile.inc: Enable UBSAN on SMM, too ......................................................................
Patch Set 1: Code-Review-2
Patch Set 1:
We don't want SMM running into UB, but I'm not sure we can easily enable this here: The sanitizers usually have some extra memory somewhere to keep track of things, and while I'm not sure if that's true for UBSAN, if it is, we ought to ensure that this memory isn't somewhere in userspace (both due to the resulting attack surface against SMM and because SMM could unwittingly corrupt non-SMM data if that stuff is simply placed anywhere)
I agree, I've yet to test this on real hardware. In any case, I could make this optional (that is, add a Kconfig option for it) and explain the risks there.
Angel Pons has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/43976 )
Change subject: lib/Makefile.inc: Enable UBSAN on SMM, too ......................................................................
Abandoned
-
Angel Pons (Code Review) -
Patrick Georgi (Code Review)