[coreboot-gerrit] Change in coreboot[master]: arch/x86: Add support for ASan to memory functions

[Patrick Georgi (Code Review)]

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

Change subject: arch/x86: Add support for ASan to memory functions
......................................................................

arch/x86: Add support for ASan to memory functions

Compiler's instrumentation cannot insert asan memory checks in
case of memory functions like memset, memcpy and memmove as they
are written in assembly.

So, we need to manually check the memory state before performing
each of these operations to ensure that ASan is triggered in case
of bad access.

Change-Id: I2030437636c77aea7cccda8efe050df4b77c15c7
Signed-off-by: Harshit Sharma 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44307
Tested-by: build bot (Jenkins) 
Reviewed-by: Werner Zeh 
---
M src/arch/x86/memcpy.c
M src/arch/x86/memmove.c
M src/arch/x86/memset.c
M src/include/asan.h
M src/lib/asan.c
5 files changed, 26 insertions(+), 1 deletion(-)

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

diff --git a/src/arch/x86/memcpy.c b/src/arch/x86/memcpy.c
index 2f23219..1cfdf89 100644
--- a/src/arch/x86/memcpy.c
+++ b/src/arch/x86/memcpy.c
@@ -1,11 +1,19 @@
 /* SPDX-License-Identifier: GPL-2.0-only */

#include <string.h>
+#include <stdbool.h>
+#include <asan.h>

void *memcpy(void *dest, const void *src, size_t n)
 {
    unsigned long d0, d1, d2;

+#if (ENV_ROMSTAGE && CONFIG(ASAN_IN_ROMSTAGE)) || \
+		(ENV_RAMSTAGE && CONFIG(ASAN_IN_RAMSTAGE))
+	check_memory_region((unsigned long)src, n, false, _RET_IP_);
+	check_memory_region((unsigned long)dest, n, true, _RET_IP_);
+#endif
+
    asm volatile(
 #ifdef __x86_64__
    	"rep ; movsd\n\t"
diff --git a/src/arch/x86/memmove.c b/src/arch/x86/memmove.c
index cdd1e8d..3ec50b2 100644
--- a/src/arch/x86/memmove.c
+++ b/src/arch/x86/memmove.c
@@ -4,12 +4,20 @@
  */

#include <string.h>
+#include <stdbool.h>
+#include <asan.h>

void *memmove(void *dest, const void *src, size_t n)
 {
    int d0, d1, d2, d3, d4, d5;
    char *ret = dest;

+#if (ENV_ROMSTAGE && CONFIG(ASAN_IN_ROMSTAGE)) || \
+		(ENV_RAMSTAGE && CONFIG(ASAN_IN_RAMSTAGE))
+	check_memory_region((unsigned long)src, n, false, _RET_IP_);
+	check_memory_region((unsigned long)dest, n, true, _RET_IP_);
+#endif
+
    __asm__ __volatile__(
    	/* Handle more 16bytes in loop */
    	"cmp $0x10, %0\n\t"
diff --git a/src/arch/x86/memset.c b/src/arch/x86/memset.c
index 1796342..fc09a9b 100644
--- a/src/arch/x86/memset.c
+++ b/src/arch/x86/memset.c
@@ -4,6 +4,8 @@

#include <string.h>
 #include <stdint.h>
+#include <stdbool.h>
+#include <asan.h>

typedef uint32_t op_t;

@@ -12,6 +14,11 @@
    int d0;
    unsigned long int dstp = (unsigned long int) dstpp;

+#if (ENV_ROMSTAGE && CONFIG(ASAN_IN_ROMSTAGE)) || \
+		(ENV_RAMSTAGE && CONFIG(ASAN_IN_RAMSTAGE))
+	check_memory_region((unsigned long)dstpp, len, true, _RET_IP_);
+#endif
+
    /* This explicit register allocation improves code very much indeed. */
    register op_t x asm("ax");

diff --git a/src/include/asan.h b/src/include/asan.h
index 8816eaf..1fe798d 100644
--- a/src/include/asan.h
+++ b/src/include/asan.h
@@ -57,6 +57,8 @@
 void asan_report(unsigned long addr, size_t size, bool is_write,
    unsigned long ip);
 void asan_init(void);
+void check_memory_region(unsigned long addr, size_t size, bool write,
+				unsigned long ret_ip);

uintptr_t __asan_shadow_offset(uintptr_t addr);
 void __asan_register_globals(struct asan_global *globals, size_t size);
diff --git a/src/lib/asan.c b/src/lib/asan.c
index 11dbf7c..6de0de1 100644
--- a/src/lib/asan.c
+++ b/src/lib/asan.c
@@ -260,7 +260,7 @@
    asan_report(addr, size, write, ret_ip);
 }

-static void check_memory_region(unsigned long addr, size_t size, bool write,
+void check_memory_region(unsigned long addr, size_t size, bool write,
    			unsigned long ret_ip)
 {
    check_memory_region_inline(addr, size, write, ret_ip);