This is the mail archive of the mailing list for the glibc project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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

On 09/05/2016 07:27 PM, Maxim Ostapenko wrote:

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
% 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. 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

. 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.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]