From a0b6493d15f891db4e68c2ceab3d74a393ef4f7b Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Mon, 12 Nov 2018 12:07:14 -0800 Subject: [PATCH 1/3] bpo-35214: Initial clang MemorySanitizer support (GH-10479) Adds configure flags for msan and ubsan builds to make it easier to enable. These also encode the detail that address sanitizer and memory sanitizer should disable pymalloc. Define MEMORY_SANITIZER when appropriate at build time and adds workarounds to existing code to mark things as initialized where the sanitizer is otherwise unable to determine that. This lets our build succeed under the memory sanitizer. not all tests pass without sanitizer failures yet but we're in pretty good shape after this. (cherry picked from commit 1584a0081500d35dc93ff88e5836df35faf3e3e2) Contributed by Gregory P. Smith [Google LLC] --- Include/Python.h | 9 ++++ .../2018-11-12-11-38-06.bpo-35214.PCHKbX.rst | 4 ++ Modules/_ctypes/callproc.c | 11 +++++ Modules/_posixsubprocess.c | 7 +++ Modules/faulthandler.c | 2 +- Python/pymath.c | 4 +- Python/random.c | 9 ++++ configure | 45 ++++++++++++++++++- configure.ac | 28 +++++++++++- 9 files changed, 115 insertions(+), 4 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-11-12-11-38-06.bpo-35214.PCHKbX.rst diff --git a/Include/Python.h b/Include/Python.h index 6177bad869c92f..565f34a371f543 100644 --- a/Include/Python.h +++ b/Include/Python.h @@ -53,6 +53,15 @@ #include "pyport.h" #include "pymacro.h" +/* A convenient way for code to know if clang's memory sanitizer is enabled. */ +#if defined(__has_feature) +# if __has_feature(memory_sanitizer) +# if !defined(MEMORY_SANITIZER) +# define MEMORY_SANITIZER +# endif +# endif +#endif + #include "pyatomic.h" /* Debug-mode build with pymalloc implies PYMALLOC_DEBUG. diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-11-12-11-38-06.bpo-35214.PCHKbX.rst b/Misc/NEWS.d/next/Core and Builtins/2018-11-12-11-38-06.bpo-35214.PCHKbX.rst new file mode 100644 index 00000000000000..c7842f331698ec --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-11-12-11-38-06.bpo-35214.PCHKbX.rst @@ -0,0 +1,4 @@ +The interpreter and extension modules have had annotations added so that +they work properly under clang's Memory Sanitizer. A new configure flag +--with-memory-sanitizer has been added to make test builds of this nature +easier to perform. diff --git a/Modules/_ctypes/callproc.c b/Modules/_ctypes/callproc.c index b8c2c746a0feb7..0e1f065e19a4ee 100644 --- a/Modules/_ctypes/callproc.c +++ b/Modules/_ctypes/callproc.c @@ -75,6 +75,10 @@ #include #endif +#ifdef MEMORY_SANITIZER +#include +#endif + #if defined(_DEBUG) || defined(__MINGW32__) /* Don't use structured exception handling on Windows if this is defined. MingW, AFAIK, doesn't support it. @@ -1136,6 +1140,13 @@ PyObject *_ctypes_callproc(PPROC pProc, rtype = _ctypes_get_ffi_type(restype); resbuf = alloca(max(rtype->size, sizeof(ffi_arg))); +#ifdef MEMORY_SANITIZER + /* ffi_call actually initializes resbuf, but from asm, which + * MemorySanitizer can't detect. Avoid false positives from MSan. */ + if (resbuf != NULL) { + __msan_unpoison(resbuf, max(rtype->size, sizeof(ffi_arg))); + } +#endif avalues = (void **)alloca(sizeof(void *) * argcount); atypes = (ffi_type **)alloca(sizeof(ffi_type *) * argcount); if (!resbuf || !avalues || !atypes) { diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index fe0e554618aabb..917a1da5e761ec 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -21,6 +21,10 @@ #include #endif +#ifdef MEMORY_SANITIZER +# include +#endif + #if defined(__ANDROID__) && __ANDROID_API__ < 21 && !defined(SYS_getdents64) # include # define SYS_getdents64 __NR_getdents64 @@ -287,6 +291,9 @@ _close_open_fds_safe(int start_fd, PyObject* py_fds_to_keep) sizeof(buffer))) > 0) { struct linux_dirent64 *entry; int offset; +#ifdef MEMORY_SANITIZER + __msan_unpoison(buffer, bytes); +#endif for (offset = 0; offset < bytes; offset += entry->d_reclen) { int fd; entry = (struct linux_dirent64 *)(buffer + offset); diff --git a/Modules/faulthandler.c b/Modules/faulthandler.c index 227424f8bbaa74..890c64577cec12 100644 --- a/Modules/faulthandler.c +++ b/Modules/faulthandler.c @@ -1393,7 +1393,7 @@ void _PyFaulthandler_Fini(void) #ifdef HAVE_SIGALTSTACK if (stack.ss_sp != NULL) { /* Fetch the current alt stack */ - stack_t current_stack; + stack_t current_stack = {}; if (sigaltstack(NULL, ¤t_stack) == 0) { if (current_stack.ss_sp == stack.ss_sp) { /* The current alt stack is the one that we installed. diff --git a/Python/pymath.c b/Python/pymath.c index 6799d200caf60f..c1606bd6d1da40 100644 --- a/Python/pymath.c +++ b/Python/pymath.c @@ -17,7 +17,9 @@ double _Py_force_double(double x) /* inline assembly for getting and setting the 387 FPU control word on gcc/x86 */ - +#ifdef MEMORY_SANITIZER +__attribute__((no_sanitize_memory)) +#endif unsigned short _Py_get_387controlword(void) { unsigned short cw; __asm__ __volatile__ ("fnstcw %0" : "=m" (cw)); diff --git a/Python/random.c b/Python/random.c index c62cbeb4ecb72a..8f6670457f3ffd 100644 --- a/Python/random.c +++ b/Python/random.c @@ -20,6 +20,10 @@ # endif #endif +#ifdef MEMORY_SANITIZER +# include +#endif + #ifdef Py_DEBUG int _Py_HashSecret_Initialized = 0; #else @@ -143,6 +147,11 @@ py_getrandom(void *buffer, Py_ssize_t size, int blocking, int raise) else { n = syscall(SYS_getrandom, dest, n, flags); } +# ifdef MEMORY_SANITIZER + if (n > 0) { + __msan_unpoison(dest, n); + } +# endif #endif if (n < 0) { diff --git a/configure b/configure index 75a80a797d347f..9b137c72603a2e 100755 --- a/configure +++ b/configure @@ -822,6 +822,8 @@ enable_optimizations with_lto with_hash_algorithm with_address_sanitizer +with_memory_sanitizer +with_undefined_behavior_sanitizer with_libs with_system_expat with_system_ffi @@ -1520,7 +1522,10 @@ Optional Packages: --with-hash-algorithm=[fnv|siphash24] select hash algorithm --with-address-sanitizer - enable AddressSanitizer + enable AddressSanitizer (asan) + --with-memory-sanitizer enable MemorySanitizer (msan) + --with-undefined-behavior-sanitizer + enable UndefinedBehaviorSanitizer (ubsan) --with-libs='lib1 ...' link against additional libs --with-system-expat build pyexpat module using an installed expat library @@ -9926,6 +9931,44 @@ if test "${with_address_sanitizer+set}" = set; then : $as_echo "$withval" >&6; } BASECFLAGS="-fsanitize=address -fno-omit-frame-pointer $BASECFLAGS" LDFLAGS="-fsanitize=address $LDFLAGS" +# ASan works by controlling memory allocation, our own malloc interferes. +with_pymalloc="no" + +else + { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5 +$as_echo "no" >&6; } +fi + + +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for --with-memory-sanitizer" >&5 +$as_echo_n "checking for --with-memory-sanitizer... " >&6; } + +# Check whether --with-memory_sanitizer was given. +if test "${with_memory_sanitizer+set}" = set; then : + withval=$with_memory_sanitizer; +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $withval" >&5 +$as_echo "$withval" >&6; } +BASECFLAGS="-fsanitize=memory -fsanitize-memory-track-origins=2 -fno-omit-frame-pointer $BASECFLAGS" +LDFLAGS="-fsanitize=memory -fsanitize-memory-track-origins=2 $LDFLAGS" +# MSan works by controlling memory allocation, our own malloc interferes. +with_pymalloc="no" + +else + { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5 +$as_echo "no" >&6; } +fi + + +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for --with-undefined-behavior-sanitizer" >&5 +$as_echo_n "checking for --with-undefined-behavior-sanitizer... " >&6; } + +# Check whether --with-undefined_behavior_sanitizer was given. +if test "${with_undefined_behavior_sanitizer+set}" = set; then : + withval=$with_undefined_behavior_sanitizer; +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $withval" >&5 +$as_echo "$withval" >&6; } +BASECFLAGS="-fsanitize=undefined $BASECFLAGS" +LDFLAGS="-fsanitize=undefined $LDFLAGS" else { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5 diff --git a/configure.ac b/configure.ac index b411c3e8f6f147..f9d406e5e812e5 100644 --- a/configure.ac +++ b/configure.ac @@ -2833,11 +2833,37 @@ esac AC_MSG_CHECKING(for --with-address-sanitizer) AC_ARG_WITH(address_sanitizer, AS_HELP_STRING([--with-address-sanitizer], - [enable AddressSanitizer]), + [enable AddressSanitizer (asan)]), [ AC_MSG_RESULT($withval) BASECFLAGS="-fsanitize=address -fno-omit-frame-pointer $BASECFLAGS" LDFLAGS="-fsanitize=address $LDFLAGS" +# ASan works by controlling memory allocation, our own malloc interferes. +with_pymalloc="no" +], +[AC_MSG_RESULT(no)]) + +AC_MSG_CHECKING(for --with-memory-sanitizer) +AC_ARG_WITH(memory_sanitizer, + AS_HELP_STRING([--with-memory-sanitizer], + [enable MemorySanitizer (msan)]), +[ +AC_MSG_RESULT($withval) +BASECFLAGS="-fsanitize=memory -fsanitize-memory-track-origins=2 -fno-omit-frame-pointer $BASECFLAGS" +LDFLAGS="-fsanitize=memory -fsanitize-memory-track-origins=2 $LDFLAGS" +# MSan works by controlling memory allocation, our own malloc interferes. +with_pymalloc="no" +], +[AC_MSG_RESULT(no)]) + +AC_MSG_CHECKING(for --with-undefined-behavior-sanitizer) +AC_ARG_WITH(undefined_behavior_sanitizer, + AS_HELP_STRING([--with-undefined-behavior-sanitizer], + [enable UndefinedBehaviorSanitizer (ubsan)]), +[ +AC_MSG_RESULT($withval) +BASECFLAGS="-fsanitize=undefined $BASECFLAGS" +LDFLAGS="-fsanitize=undefined $LDFLAGS" ], [AC_MSG_RESULT(no)]) From 7a683400f82b388c49927687ccf11f0de47fbd73 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google]" Date: Mon, 12 Nov 2018 13:52:49 -0800 Subject: [PATCH 2/3] [3.6] bpo-35214: Initial clang MemorySanitizer support (GH-10479) Adds configure flags for msan and ubsan builds to make it easier to enable. These also encode the detail that address sanitizer and memory sanitizer should disable pymalloc. Define MEMORY_SANITIZER when appropriate at build time and adds workarounds to existing code to mark things as initialized where the sanitizer is otherwise unable to determine that. This lets our build succeed under the memory sanitizer. not all tests pass without sanitizer failures yet but we're in pretty good shape after this.. (cherry picked from commit 1584a0081500d35dc93ff88e5836df35faf3e3e2) Co-authored-by: Gregory P. Smith From 6aef90e746889d11060f917f464ca2e101eb4e76 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google]" Date: Mon, 12 Nov 2018 14:24:05 -0800 Subject: [PATCH 3/3] whitespace fix from make patchcheck. unrelated to the main change, but makes the CI happy so why not do it now. --- Modules/_posixsubprocess.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index 917a1da5e761ec..fe519dea9c5475 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -798,11 +798,11 @@ static PyMethodDef module_methods[] = { static struct PyModuleDef _posixsubprocessmodule = { - PyModuleDef_HEAD_INIT, - "_posixsubprocess", - module_doc, - -1, /* No memory is needed. */ - module_methods, + PyModuleDef_HEAD_INIT, + "_posixsubprocess", + module_doc, + -1, /* No memory is needed. */ + module_methods, }; PyMODINIT_FUNC