This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH BZ#20422] Do not allow asan/msan/tsan and fortify at the same time.
- From: Florian Weimer <fweimer at redhat dot com>
- To: Maxim Ostapenko <m dot ostapenko at samsung dot com>, GNU C Library <libc-alpha at sourceware dot org>
- Cc: Kostya Serebryany <kcc at google dot com>, Yuri Gribov <tetra2005 at gmail dot com>
- Date: Tue, 6 Sep 2016 10:39:11 +0200
- Subject: Re: [PATCH BZ#20422] Do not allow asan/msan/tsan and fortify at the same time.
- Authentication-results: sourceware.org; auth=none
- References: <57CDAB08.8060601@samsung.com>
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.
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.
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