Change in coreboot[master]: lib: Update ASan compiler flags and Kconfig help text

[Harshit Sharma (Code Review)]

Harshit Sharma has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43164 )

Change subject: lib: Update ASan compiler flags and Kconfig help text
......................................................................

lib: Update ASan compiler flags and Kconfig help text

An additional compiler flag is added to make use of the shadow
offset callback feature we introduced in our GCC patch. Also,
the help text of ASAN is updated to tell user that this GCC patch
needs to be applied in order to use ASan.

Change-Id: Ia187e4991bf808f4ae137eff0ffdb9baea0085e9
Signed-off-by: Harshit Sharma 
---
M src/Kconfig
M src/lib/Makefile.inc
2 files changed, 4 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/43164/1

diff --git a/src/Kconfig b/src/Kconfig
index 1eac469..6c8fe3e 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -259,6 +259,8 @@
      designed to find out-of-bounds accesses.
      This feature consumes up to 1/8 of available memory and brings about
      ~1.5x performance slowdown.
+	  (Before using this feature, make sure that asan_shadow_offset_callback
+	  patch is applied to GCC.)
      If unsure, say N.

config RELOCATABLE_RAMSTAGE
diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc
index cc8ac78..93d6ab1 100644
--- a/src/lib/Makefile.inc
+++ b/src/lib/Makefile.inc
@@ -9,7 +9,8 @@

ifeq ($(CONFIG_ASAN),y)
 ramstage-y += asan.c
-CFLAGS_asan += -fsanitize=kernel-address \
+# Ensure that asan_shadow_offset_callback patch is applied to GCC before ASan is used.
+CFLAGS_asan += -fsanitize=kernel-address --param asan-use-shadow-offset-callback=1 \
    	--param asan-stack=1 --param asan-globals=1 \
    	--param asan-instrumentation-with-call-threshold=0
 CFLAGS_ramstage += $(CFLAGS_asan)

[Harshit Sharma (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Werner Zeh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/43164

to look at the new patch set (#3).

Change subject: lib: Update ASan compiler flags and Kconfig help text
......................................................................

lib: Update ASan compiler flags and Kconfig help text

An additional compiler flag is added to make use of the shadow
offset callback feature we introduced in our GCC patch. Also,
the help text of ASAN is updated to tell user that this GCC patch
needs to be applied in order to use ASan.

Change-Id: Ia187e4991bf808f4ae137eff0ffdb9baea0085e9
Signed-off-by: Harshit Sharma 
---
M src/Kconfig
M src/lib/Makefile.inc
M src/lib/asan.c
3 files changed, 14 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/43164/3

[Harshit Sharma (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Werner Zeh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/43164

to look at the new patch set (#4).

Change subject: lib: Update ASan compiler flags and Kconfig help text
......................................................................

lib: Update ASan compiler flags and Kconfig help text

An additional compiler flag is added to make use of the shadow
offset callback feature we introduced in our GCC patch. Also,
the help text of ASAN is updated to tell user that this GCC patch
needs to be applied in order to use ASan.

Change-Id: Ia187e4991bf808f4ae137eff0ffdb9baea0085e9
Signed-off-by: Harshit Sharma 
---
M src/Kconfig
M src/lib/Makefile.inc
2 files changed, 5 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/43164/4

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43164 )

Change subject: lib: Update ASan compiler flags and Kconfig help text
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43164/4/src/Kconfig 
File src/Kconfig:

https://review.coreboot.org/c/coreboot/+/43164/4/src/Kconfig@284 
PS4, Line 284: Before using this feature, make sure that asan_shadow_offset_callback
             : 	  patch is applied to GCC.
I'd make this a Kconfig comment that pops up when selecting ASAN. I forgot about it.

[Harshit Sharma (Code Review)]

Harshit Sharma has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43164 )

Change subject: lib: Update ASan compiler flags and Kconfig help text
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43164/4/src/Kconfig 
File src/Kconfig:

https://review.coreboot.org/c/coreboot/+/43164/4/src/Kconfig@284 
PS4, Line 284: Before using this feature, make sure that asan_shadow_offset_callback
             : 	  patch is applied to GCC.
> I'd make this a Kconfig comment that pops up when selecting ASAN. I forgot about it.

It would be great to have a pop-up message. But can we create pop-ups in Kconfig?

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43164 )

Change subject: lib: Update ASan compiler flags and Kconfig help text
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43164/4/src/Kconfig 
File src/Kconfig:

https://review.coreboot.org/c/coreboot/+/43164/4/src/Kconfig@284 
PS4, Line 284: Before using this feature, make sure that asan_shadow_offset_callback
             : 	  patch is applied to GCC.
> It would be great to have a pop-up message. […]

As I said, make a Kconfig comment that pops up when selecting ASAN. This woulld look more or less like this:

if ASAN_IN_RAMSTAGE
 comment "Before using this feature, ..."
 endif

[Harshit Sharma (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Werner Zeh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/43164

to look at the new patch set (#5).

Change subject: lib: Update ASan compiler flags and Kconfig help text
......................................................................

lib: Update ASan compiler flags and Kconfig help text

An additional compiler flag is added to make use of the shadow
offset callback feature we introduced in our GCC patch. Also,
a comment is added to tell user that this GCC patch needs to be
applied in order to use ASan.

Change-Id: Ia187e4991bf808f4ae137eff0ffdb9baea0085e9
Signed-off-by: Harshit Sharma 
---
M src/Kconfig
M src/lib/Makefile.inc
2 files changed, 7 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/43164/5

[Harshit Sharma (Code Review)]

Harshit Sharma has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43164 )

Change subject: lib: Update ASan compiler flags and Kconfig help text
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43164/4/src/Kconfig 
File src/Kconfig:

https://review.coreboot.org/c/coreboot/+/43164/4/src/Kconfig@284 
PS4, Line 284: Before using this feature, make sure that asan_shadow_offset_callback
             : 	  patch is applied to GCC.
> As I said, make a Kconfig comment that pops up when selecting ASAN. […]

Done. Thanks.

[Harshit Sharma (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Werner Zeh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/43164

to look at the new patch set (#6).

Change subject: lib: Update ASan compiler flags and Kconfig help text
......................................................................

lib: Update ASan compiler flags and Kconfig help text

An additional compiler flag is added to make use of the shadow
offset callback feature we introduced in our GCC patch. Also,
a comment is added to tell user that this GCC patch needs to be
applied in order to use ASan.

Change-Id: Ia187e4991bf808f4ae137eff0ffdb9baea0085e9
Signed-off-by: Harshit Sharma 
---
M src/Kconfig
M src/lib/Makefile.inc
2 files changed, 7 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/43164/6

[Werner Zeh (Code Review)]

Werner Zeh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43164 )

Change subject: lib: Update ASan compiler flags and Kconfig help text
......................................................................

Patch Set 6: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43164 )

Change subject: lib: Update ASan compiler flags and Kconfig help text
......................................................................

Patch Set 6:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43164/6/src/Kconfig 
File src/Kconfig:

https://review.coreboot.org/c/coreboot/+/43164/6/src/Kconfig@287 
PS6, Line 287:            
why have these spaces?

[Harshit Sharma (Code Review)]

Harshit Sharma has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43164 )

Change subject: lib: Update ASan compiler flags and Kconfig help text
......................................................................

Patch Set 6:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43164/6/src/Kconfig 
File src/Kconfig:

https://review.coreboot.org/c/coreboot/+/43164/6/src/Kconfig@287 
PS6, Line 287:
> why have these spaces?

Since there is no option to add multi-line comments, I added those extra spaces so that it doesn't look like two separate comments.

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43164 )

Change subject: lib: Update ASan compiler flags and Kconfig help text
......................................................................

lib: Update ASan compiler flags and Kconfig help text

An additional compiler flag is added to make use of the shadow
offset callback feature we introduced in our GCC patch. Also,
a comment is added to tell user that this GCC patch needs to be
applied in order to use ASan.

Change-Id: Ia187e4991bf808f4ae137eff0ffdb9baea0085e9
Signed-off-by: Harshit Sharma 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/43164
Tested-by: build bot (Jenkins) 
Reviewed-by: Werner Zeh 
---
M src/Kconfig
M src/lib/Makefile.inc
2 files changed, 7 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Werner Zeh: Looks good to me, approved

diff --git a/src/Kconfig b/src/Kconfig
index 1540c56..f8af522 100644
--- a/src/Kconfig
+++ b/src/Kconfig
@@ -283,6 +283,11 @@

If unsure, say N.

+if ASAN_IN_RAMSTAGE
+	comment "Before using this feature, make sure that           "
+	comment "asan_shadow_offset_callback patch is applied to GCC."
+endif
+
 choice
    prompt "Stage Cache for ACPI S3 resume"
    default NO_STAGE_CACHE if !HAVE_ACPI_RESUME
diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc
index 0b43427..e33a449 100644
--- a/src/lib/Makefile.inc
+++ b/src/lib/Makefile.inc
@@ -9,7 +9,8 @@

ifeq ($(CONFIG_ASAN_IN_RAMSTAGE),y)
 ramstage-y += asan.c
-CFLAGS_asan += -fsanitize=kernel-address \
+# Ensure that asan_shadow_offset_callback patch is applied to GCC before ASan is used.
+CFLAGS_asan += -fsanitize=kernel-address --param asan-use-shadow-offset-callback=1 \
    	--param asan-stack=1 --param asan-globals=1 \
    	--param asan-instrumentation-with-call-threshold=0 \
    	-fsanitize-address-use-after-scope \

[9elements QA (Code Review)]

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43164 )

Change subject: lib: Update ASan compiler flags and Kconfig help text
......................................................................

Patch Set 7:

Automatic boot test returned (PASS/FAIL/TOTAL): 6/1/7
"QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/16036
"QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/16035
"QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/16034
"QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/16033
"QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/16032
"HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/16038
"HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/16037

Please note: This test is under development and might not be accurate at all!