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

[Maxim Ostapenko]

On 06/09/16 11:39, Florian Weimer wrote:
> On 09/05/2016 07:27 PM, Maxim Ostapenko wrote:
>> 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.
>
> Why does the above cause a warning, but this does not, and happily 
> prints the undefined array members?
>
> #include <stdio.h>
> #include <pwd.h>
> #include <unistd.h>
> #include <string.h>
> #include <err.h>
> #include <grp.h>
>
> int main()
> {
>   struct passwd *pw = getpwuid (getuid ());
>   if (pw == NULL)
>     err (1, "getpwuid");
>   gid_t groups[50];
>   int ngroups = 50;
>   if (getgrouplist (pw->pw_name, pw->pw_gid, groups, &ngroups) < 0)
>     errx (1, "getgrouplist");
>   for (int i = 0; i < 50; ++i)
>     printf ("group %d: %lld\n", i + 1, (long long) groups[i]);
> }
>
>
> I find this rather confusing.  -D_FORTIFY_SOURCE=2 does not make a 
> difference here.


I think because MSan doesn't have getgrouplist interceptor.

>
>> 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.
>
> At least for Memory Sanitizer, the documentation clearly says that 
> *everything* has to be compiled with it.  I read that as as meaning 
> that the interceptors are just a kludge.

I thought we should compile everything except Glibc, no? Compiling Glibc 
with MSan would be tricky, that's why it needs interceptors.

>
> If you do not intended to implement further interceptors, I expect you 
> don't want to implement those for open/openat either, other 
> compile-time fortification, or whatever other fortify functions we 
> might add which are not directly related to memory safety.  This means 
> that for full coverage, a developer would have to compile to test with 
> the different sanitizers *and* _FORTIFY_SOURCE.  I'm not sure if that 
> leads to a good developer experience.
>
> We could conceivably guard every _chk wrapper (say __sprintf_chk) with
>
> #ifndef __fortify_no___sprintf_chk
> …
> #endif
>
> .  In sanitizer mode, for those wrappers you have deemed to be 
> unnecessary, the compiler would define these macros, so that glibc 
> wouldn't install the wrapper.
>
> Another option would be to provide a glibc which has been compiled 
> with the required sanitizers, so that most interceptors become 
> unnecessary.
>
>> 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?
>
> It does not.  Carlos proposed something earlier this year, but it was 
> not able to check for the presence expected warnings.  We need 
> something like this for the compile-time checks.
>
> Florian
>
>