From 05b3c6beca1497a74ef4c6271c8f48997369b0ab Mon Sep 17 00:00:00 2001 From: Douglas Katzman Date: Tue, 31 Oct 2017 10:54:46 -0400 Subject: [PATCH] Allow calling C code that was compiled with -fsanitize=memory * Mark all results from malloc as nonpoisoned. This is a remediation in lieu of marking the allocated memory when and only when written. * Mark all dynamic-extent vectors as nonpoisoned. To enable, :MSAN must be present in *FEATURES*, and the C runtime must be compiled with -DMEMORY_SANITIZER. Only works on x86-64/linux. --- src/code/linkage-table.lisp | 14 +++++++------- src/compiler/early-c.lisp | 1 + src/compiler/main.lisp | 8 ++++++++ src/compiler/target-main.lisp | 3 +++ src/compiler/x86-64/alloc.lisp | 20 ++++++++++++++++++++ src/runtime/immobile-space.c | 2 +- src/runtime/os-common.c | 25 ++++++++++++++++++++++++- 7 files changed, 64 insertions(+), 9 deletions(-) diff --git a/src/code/linkage-table.lisp b/src/code/linkage-table.lisp index 51eb92508..b26b38e16 100644 --- a/src/code/linkage-table.lisp +++ b/src/code/linkage-table.lisp @@ -69,10 +69,10 @@ (defun update-linkage-table () (dohash ((key table-address) *linkage-info* :locked t) (let* ((datap (listp key)) - (name (if datap (car key) key)) - (real-address - (ensure-dynamic-foreign-symbol-address name datap))) - (aver (and table-address real-address)) - (write-linkage-table-entry table-address - real-address - datap)))) + (name (if datap (car key) key))) + ;; Absent a fix for the issue noted above at "Should figure out ...", + ;; never ever re-touch malloc or free if already linked. + (unless (or #!+sb-dynamic-core (member name '("malloc" "free") :test 'string=)) + (let ((real-address (ensure-dynamic-foreign-symbol-address name datap))) + (aver (and table-address real-address)) + (write-linkage-table-entry table-address real-address datap)))))) diff --git a/src/compiler/early-c.lisp b/src/compiler/early-c.lisp index bb8176365..70cba0454 100644 --- a/src/compiler/early-c.lisp +++ b/src/compiler/early-c.lisp @@ -132,6 +132,7 @@ (defvar *warnings-p*) (defvar *lambda-conversions*) (defvar *compile-object* nil) +(defvar *msan-compatible-stack-unpoison* nil) (defvar *stack-allocate-dynamic-extent* t "If true (the default), the compiler respects DYNAMIC-EXTENT declarations diff --git a/src/compiler/main.lisp b/src/compiler/main.lisp index 9abfe5e50..0ce251e91 100644 --- a/src/compiler/main.lisp +++ b/src/compiler/main.lisp @@ -1701,6 +1701,14 @@ necessary, since type inference may take arbitrarily long to converge.") (*macro-policy* *macro-policy*) (*compiler-coverage-metadata* (cons (make-hash-table :test 'equal) (make-hash-table :test 'equal))) + ;; Whether to emit msan unpoisoning code depends on the runtime + ;; value of the feature, not "#+msan", because we can use the target + ;; compiler to compile code for itself which isn't sanitized, + ;; *or* code for another image which is sanitized. + ;; And we can also cross-compile assuming msan. + (*msan-compatible-stack-unpoison* + (member :msan #+sb-xc-host sb-cold::*shebang-features* + #-sb-xc-host *features*)) (*handled-conditions* *handled-conditions*) (*disabled-package-locks* *disabled-package-locks*) (*lexenv* (make-null-lexenv)) diff --git a/src/compiler/target-main.lisp b/src/compiler/target-main.lisp index dd1678b6c..bb4a5ec9a 100644 --- a/src/compiler/target-main.lisp +++ b/src/compiler/target-main.lisp @@ -42,6 +42,9 @@ (*block-compile* nil) (*allow-instrumenting* nil) (*compiler-coverage-metadata* nil) + (*msan-compatible-stack-unpoison* + (and (member :msan *features*) + (find-dynamic-foreign-symbol-address "__msan_unpoison"))) (*current-path* nil) (*last-format-string* nil) (*last-format-args* nil) diff --git a/src/compiler/x86-64/alloc.lisp b/src/compiler/x86-64/alloc.lisp index 81c935929..b20170ba8 100644 --- a/src/compiler/x86-64/alloc.lisp +++ b/src/compiler/x86-64/alloc.lisp @@ -150,6 +150,26 @@ ;; It would also be good to skip zero-fill of specialized vectors ;; perhaps in a policy-dependent way. At worst you'd see random ;; bits, and CLHS says consequences are undefined. + (when sb!c::*msan-compatible-stack-unpoison* + ;; Unpoison all DX vectors regardless of widetag. + ;; Mark the header and length as valid, not just the payload. + #!+linux ; shadow space differs by OS + (progn + ;; from 'llvm/projects/compiler-rt/lib/msan/msan.h': + ;; "#define MEM_TO_SHADOW(mem) (((uptr)(mem)) ^ 0x500000000000ULL)" + (inst mov rax #x500000000000) + (inst lea rdi (make-ea :qword :base result + :disp (- other-pointer-lowtag))) + (inst xor rdi rax) ; compute shadow address + (zeroize rax) + (cond ((sc-is words immediate) + (inst mov rcx (tn-value words))) + (t + (move rcx words) + (inst shr rcx n-fixnum-tag-bits))) + (inst add rcx sb!vm:vector-data-offset) + (inst rep) + (inst stos rax))) (let ((data-addr (make-ea :qword :base result :disp (- (* vector-data-offset n-word-bytes) diff --git a/src/runtime/immobile-space.c b/src/runtime/immobile-space.c index e24df76a6..92d3e6f99 100644 --- a/src/runtime/immobile-space.c +++ b/src/runtime/immobile-space.c @@ -409,7 +409,7 @@ enliven_immobile_obj(lispobj *ptr, int rescan) // a native pointer if (pointerish) { if (page_index >= FIRST_VARYOBJ_PAGE) varyobj_page_touched_bits[(page_index-FIRST_VARYOBJ_PAGE)/32] - |= 1 << (page_index & 31); + |= 1U << (page_index & 31); else SET_WP_FLAG(page_index, WRITE_PROTECT_CLEARED); } diff --git a/src/runtime/os-common.c b/src/runtime/os-common.c index c7aca79c9..3ea83a966 100644 --- a/src/runtime/os-common.c +++ b/src/runtime/os-common.c @@ -12,6 +12,9 @@ #include #include #include +#ifdef MEMORY_SANITIZER +#include +#endif #include "sbcl.h" #include "globals.h" @@ -158,6 +161,21 @@ os_dlsym_default(char *name) } #endif +#ifdef MEMORY_SANITIZER +/* Unless the Lisp compiler annotates every (SETF SAP-REF-n) to update + * shadow memory indicating non-poison state byte-for-byte, + * we need to unpoison all malloc() results, + * otherwise the memory can't be read by sanitized code. + */ +static void* malloc_unpoisoned(size_t size) +{ + void* result = malloc(size); + if (result) + __msan_unpoison(result, size); + return result; +} +#endif + void os_link_runtime() { extern void write_protect_immobile_space(); @@ -179,7 +197,12 @@ void os_link_runtime() datap = lowtag_of(item) == LIST_POINTER_LOWTAG; symbol_name = datap ? CONS(item)->car : item; namechars = (void*)(intptr_t)(VECTOR(symbol_name)->data); - result = os_dlsym_default(namechars); +#ifdef MEMORY_SANITIZER + if (!strcmp(namechars,"malloc")) + result = malloc_unpoisoned; + else +#endif + result = os_dlsym_default(namechars); odxprint(runtime_link, "linking %s => %p", namechars, result); if (link_target == validated_end) { -- 2.11.4.GIT