[PATCH BZ#20422] Do not allow asan/msan/tsan and fortify at the same time.

[Maxim Ostapenko <m.ostapenko@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 3395 bytes --]

Hi!

When fortify is used with MSan it will cause MSan false positives.

#include <stdio.h>
#include <string.h>
int main()
{
         char text[100];
         sprintf(text, "hello");
         printf("%lu\n", strlen(text));
}

% clang test.c -fsanitize=memory   -O3 && ./a.out
5
% clang test.c -fsanitize=memory -D_FORTIFY_SOURCE=2  -O3 && ./a.out
Uninitialized bytes in __interceptor_strlen at offset 0 inside 
[0x7ffe259e4d20, 6)
==26297==WARNING: MemorySanitizer: use-of-uninitialized-value
     #0 0x4869cc in main

With ASan, this will not cause false positives, but may case false 
negatives or just confuse people with "wrong" reports when fortify 
catches the error.

Although fortify is good thing as it (and it's enabled by default on 
some major distros e.g. Ubuntu and Gentoo), people still complain about 
{A, M}San vs fortify interaction, see e.g. 
https://github.com/google/sanitizers/issues/689. One possible solution 
would be to extend {A, M}San to support foo_chk() functions, but this 
would increase the complexity of sanitizer tools with quite small 
benefit. Another choice would be to warn users when they compile their 
code with {A, M, T}San and fortify enabled.

This patch implements the second approach. The simplest way to warn is 
to modify the Glibc headers to check if fortify and one of the 
sanitizers is enabled. Does this look reasonable?

I've tried to add a testcase for new warning into Glibc testsuite, but 
failed to see how exactly I can do it. Does Glibc have some framework 
for compilation tests? Could someone help me with this issue?
For now, I've tested this patch locally with GCC 4.8, fresh GCC and 
fresh Clang on my Ubuntu box:

gcc test.c -fsanitize=address -D_FORTIFY_SOURCE=2  -O3 
-L${SYSROOT}/usr/lib -I${SYSROOT}/include -Wl,-rpath=${SYSROOT}/lib 
-Wl,--dynamic-linker=${SYSROOT}/lib/ld-2.24.90.so  -S
In file included from 
/home/max/install/glibc//include/bits/libc-header-start.h:33:0,
                  from /home/max/install/glibc//include/stdio.h:28,
                  from test.c:1:
/home/max/install/glibc//include/features.h:374:3: warning: #warning 
_FORTIFY_SOURCE is not compatible with sanitizer [-Wcpp]
  # warning _FORTIFY_SOURCE is not compatible with sanitizer


~/install/master/bin/gcc test.c -fsanitize=address -D_FORTIFY_SOURCE=2  
-O3 -L${SYSROOT}/usr/lib -I${SYSROOT}/include -Wl,-rpath=${SYSROOT}/lib 
-Wl,--dynamic-linker=${SYSROOT}/lib/ld-2.24.90.so  -S
In file included from 
/home/max/install/glibc//include/bits/libc-header-start.h:33:0,
                  from /home/max/install/glibc//include/stdio.h:28,
                  from test.c:1:
/home/max/install/glibc//include/features.h:374:3: warning: #warning 
_FORTIFY_SOURCE is not compatible with sanitizer [-Wcpp]
  # warning _FORTIFY_SOURCE is not compatible with sanitizer

clang  test.c -fsanitize=address -D_FORTIFY_SOURCE=2  -O3 
-L${SYSROOT}/usr/lib -I${SYSROOT}/include -Wl,-rpath=${SYSROOT}/lib 
-Wl,--dynamic-linker=${SYSROOT}/lib/ld-2.24.90.so
In file included from test.c:1:
In file included from /home/max/install/glibc//include/stdio.h:28:
In file included from 
/home/max/install/glibc//include/bits/libc-header-start.h:33:
/home/max/install/glibc//include/features.h:374:3: warning: 
_FORTIFY_SOURCE is not compatible with sanitizer [-W#warnings]
# warning _FORTIFY_SOURCE is not compatible with sanitizer
   ^
1 warning generated.


-Maxim

[-- Attachment #2: fortify-asan.diff --]
[-- Type: text/x-diff, Size: 2289 bytes --]

commit b153492ed25b1bc70141566e6644897b89a82e26
Author: Maxim Ostapenko <m.ostapenko@samsung.com>
Date:   Wed Aug 31 15:29:49 2016 +0300

    Do not allow asan/msan/tsan and fortify at the same time.

    When fortify is used with msan it will cause msan false positives. This is
    happening because fortify replaces libc functions (e.g. sprintf) with its own
    variants (__sprintf_chk) and the sanitizers don't know about these variants.
    Supporting fortify in *san makes little sense because fortify does not add
    anything to the sanitizers and it will only increase the complexity. So, the
    better way is to warn the user that the sanitizers and fortify are
    incompatible.

	[BZ #20422]
	* include/features.h (__SANITIZER_ENABLED): New macros. Define it if
	compiler has {A, T, M}San enabled. Warn if __SANITIZER_ENABLED and
	_FORTIFY_SOURCE are both defined.

diff --git a/ChangeLog b/ChangeLog
index d24536b..ee513cd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2016-09-05  Maxim Ostapenko  <m.ostapenko@samsung.com>
+
+	[BZ #20422]
+	* include/features.h (__SANITIZER_ENABLED): New macros. Define it if
+	compiler has {A, T, M}San enabled. Warn if __SANITIZER_ENABLED and
+	_FORTIFY_SOURCE are both defined.
+
 2016-08-30  Svante Signell  <svante.signell@gmail.com>
 
 	* sysdeps/mach/hurd/adjtime.c (__adjtime): When OLDDELTA is NULL, make
diff --git a/include/features.h b/include/features.h
index 650d4c5..1158e5a 100644
--- a/include/features.h
+++ b/include/features.h
@@ -355,11 +355,23 @@
 # define __USE_REENTRANT	1
 #endif
 
+#if !defined(__has_feature)
+# define __has_feature(x) 0
+#endif
+
+#if defined __SANITIZE_ADDRESS__ || defined __SANITIZE_THREAD__		    \
+    || __has_feature(address_sanitizer) || __has_feature(thread_sanitizer)  \
+    || __has_feature(memory_sanitizer)
+# define __SANITIZER_ENABLED 1
+#endif
+
 #if defined _FORTIFY_SOURCE && _FORTIFY_SOURCE > 0
 # if !defined __OPTIMIZE__ || __OPTIMIZE__ <= 0
 #  warning _FORTIFY_SOURCE requires compiling with optimization (-O)
 # elif !__GNUC_PREREQ (4, 1)
 #  warning _FORTIFY_SOURCE requires GCC 4.1 or later
+# elif defined __SANITIZER_ENABLED
+# warning _FORTIFY_SOURCE is not compatible with sanitizer
 # elif _FORTIFY_SOURCE > 1
 #  define __USE_FORTIFY_LEVEL 2
 # else