[compiler-rt] r353351 - [sanitizer] Don't unpoison buffer in getpw/getgr functions
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 6 16:08:15 PST 2019
Author: vitalybuka
Date: Wed Feb 6 16:08:14 2019
New Revision: 353351
URL: http://llvm.org/viewvc/llvm-project?rev=353351&view=rev
Log:
[sanitizer] Don't unpoison buffer in getpw/getgr functions
Summary:
Buffer should be referenced by results so used parts will be unpoisoned with unpoison_group and unpoison_passwd.
This fixes TSAN performance issue made us to disable this interceptors.
Reviewers: eugenis, dvyukov
Subscribers: srhines, kubamracek, krytarowski, #sanitizers
Tags: #sanitizers
Differential Revision: https://reviews.llvm.org/D57731
Added:
compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/getpw_getgr.cc
Modified:
compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc?rev=353351&r1=353350&r2=353351&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc Wed Feb 6 16:08:14 2019
@@ -1822,27 +1822,27 @@ UNUSED static void unpoison_passwd(void
if (pwd) {
COMMON_INTERCEPTOR_WRITE_RANGE(ctx, pwd, sizeof(*pwd));
if (pwd->pw_name)
- COMMON_INTERCEPTOR_INITIALIZE_RANGE(pwd->pw_name,
- REAL(strlen)(pwd->pw_name) + 1);
+ COMMON_INTERCEPTOR_WRITE_RANGE(ctx, pwd->pw_name,
+ REAL(strlen)(pwd->pw_name) + 1);
if (pwd->pw_passwd)
- COMMON_INTERCEPTOR_INITIALIZE_RANGE(pwd->pw_passwd,
- REAL(strlen)(pwd->pw_passwd) + 1);
+ COMMON_INTERCEPTOR_WRITE_RANGE(ctx, pwd->pw_passwd,
+ REAL(strlen)(pwd->pw_passwd) + 1);
#if !SANITIZER_ANDROID
if (pwd->pw_gecos)
- COMMON_INTERCEPTOR_INITIALIZE_RANGE(pwd->pw_gecos,
- REAL(strlen)(pwd->pw_gecos) + 1);
+ COMMON_INTERCEPTOR_WRITE_RANGE(ctx, pwd->pw_gecos,
+ REAL(strlen)(pwd->pw_gecos) + 1);
#endif
-#if SANITIZER_MAC
+#if SANITIZER_MAC || SANITIZER_FREEBSD || SANITIZER_NETBSD || SANITIZER_OPENBSD
if (pwd->pw_class)
- COMMON_INTERCEPTOR_INITIALIZE_RANGE(pwd->pw_class,
- REAL(strlen)(pwd->pw_class) + 1);
+ COMMON_INTERCEPTOR_WRITE_RANGE(ctx, pwd->pw_class,
+ REAL(strlen)(pwd->pw_class) + 1);
#endif
if (pwd->pw_dir)
- COMMON_INTERCEPTOR_INITIALIZE_RANGE(pwd->pw_dir,
- REAL(strlen)(pwd->pw_dir) + 1);
+ COMMON_INTERCEPTOR_WRITE_RANGE(ctx, pwd->pw_dir,
+ REAL(strlen)(pwd->pw_dir) + 1);
if (pwd->pw_shell)
- COMMON_INTERCEPTOR_INITIALIZE_RANGE(pwd->pw_shell,
- REAL(strlen)(pwd->pw_shell) + 1);
+ COMMON_INTERCEPTOR_WRITE_RANGE(ctx, pwd->pw_shell,
+ REAL(strlen)(pwd->pw_shell) + 1);
}
}
@@ -1850,17 +1850,17 @@ UNUSED static void unpoison_group(void *
if (grp) {
COMMON_INTERCEPTOR_WRITE_RANGE(ctx, grp, sizeof(*grp));
if (grp->gr_name)
- COMMON_INTERCEPTOR_INITIALIZE_RANGE(grp->gr_name,
- REAL(strlen)(grp->gr_name) + 1);
+ COMMON_INTERCEPTOR_WRITE_RANGE(ctx, grp->gr_name,
+ REAL(strlen)(grp->gr_name) + 1);
if (grp->gr_passwd)
- COMMON_INTERCEPTOR_INITIALIZE_RANGE(grp->gr_passwd,
- REAL(strlen)(grp->gr_passwd) + 1);
+ COMMON_INTERCEPTOR_WRITE_RANGE(ctx, grp->gr_passwd,
+ REAL(strlen)(grp->gr_passwd) + 1);
char **p = grp->gr_mem;
for (; *p; ++p) {
- COMMON_INTERCEPTOR_INITIALIZE_RANGE(*p, REAL(strlen)(*p) + 1);
+ COMMON_INTERCEPTOR_WRITE_RANGE(ctx, *p, REAL(strlen)(*p) + 1);
}
- COMMON_INTERCEPTOR_INITIALIZE_RANGE(grp->gr_mem,
- (p - grp->gr_mem + 1) * sizeof(*p));
+ COMMON_INTERCEPTOR_WRITE_RANGE(ctx, grp->gr_mem,
+ (p - grp->gr_mem + 1) * sizeof(*p));
}
}
#endif // SANITIZER_POSIX
@@ -1916,10 +1916,8 @@ INTERCEPTOR(int, getpwnam_r, const char
// its metadata. See
// https://github.com/google/sanitizers/issues/321.
int res = REAL(getpwnam_r)(name, pwd, buf, buflen, result);
- if (!res) {
- if (result && *result) unpoison_passwd(ctx, *result);
- COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, buflen);
- }
+ if (!res && result && *result)
+ unpoison_passwd(ctx, *result);
if (result) COMMON_INTERCEPTOR_WRITE_RANGE(ctx, result, sizeof(*result));
return res;
}
@@ -1931,10 +1929,8 @@ INTERCEPTOR(int, getpwuid_r, u32 uid, __
// its metadata. See
// https://github.com/google/sanitizers/issues/321.
int res = REAL(getpwuid_r)(uid, pwd, buf, buflen, result);
- if (!res) {
- if (result && *result) unpoison_passwd(ctx, *result);
- COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, buflen);
- }
+ if (!res && result && *result)
+ unpoison_passwd(ctx, *result);
if (result) COMMON_INTERCEPTOR_WRITE_RANGE(ctx, result, sizeof(*result));
return res;
}
@@ -1947,10 +1943,8 @@ INTERCEPTOR(int, getgrnam_r, const char
// its metadata. See
// https://github.com/google/sanitizers/issues/321.
int res = REAL(getgrnam_r)(name, grp, buf, buflen, result);
- if (!res) {
- if (result && *result) unpoison_group(ctx, *result);
- COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, buflen);
- }
+ if (!res && result && *result)
+ unpoison_group(ctx, *result);
if (result) COMMON_INTERCEPTOR_WRITE_RANGE(ctx, result, sizeof(*result));
return res;
}
@@ -1962,10 +1956,8 @@ INTERCEPTOR(int, getgrgid_r, u32 gid, __
// its metadata. See
// https://github.com/google/sanitizers/issues/321.
int res = REAL(getgrgid_r)(gid, grp, buf, buflen, result);
- if (!res) {
- if (result && *result) unpoison_group(ctx, *result);
- COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, buflen);
- }
+ if (!res && result && *result)
+ unpoison_group(ctx, *result);
if (result) COMMON_INTERCEPTOR_WRITE_RANGE(ctx, result, sizeof(*result));
return res;
}
@@ -2031,10 +2023,8 @@ INTERCEPTOR(int, getpwent_r, __sanitizer
// its metadata. See
// https://github.com/google/sanitizers/issues/321.
int res = REAL(getpwent_r)(pwbuf, buf, buflen, pwbufp);
- if (!res) {
- if (pwbufp && *pwbufp) unpoison_passwd(ctx, *pwbufp);
- COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, buflen);
- }
+ if (!res && pwbufp && *pwbufp)
+ unpoison_passwd(ctx, *pwbufp);
if (pwbufp) COMMON_INTERCEPTOR_WRITE_RANGE(ctx, pwbufp, sizeof(*pwbufp));
return res;
}
@@ -2046,10 +2036,8 @@ INTERCEPTOR(int, getgrent_r, __sanitizer
// its metadata. See
// https://github.com/google/sanitizers/issues/321.
int res = REAL(getgrent_r)(pwbuf, buf, buflen, pwbufp);
- if (!res) {
- if (pwbufp && *pwbufp) unpoison_group(ctx, *pwbufp);
- COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, buflen);
- }
+ if (!res && pwbufp && *pwbufp)
+ unpoison_group(ctx, *pwbufp);
if (pwbufp) COMMON_INTERCEPTOR_WRITE_RANGE(ctx, pwbufp, sizeof(*pwbufp));
return res;
}
@@ -2069,10 +2057,8 @@ INTERCEPTOR(int, fgetpwent_r, void *fp,
// its metadata. See
// https://github.com/google/sanitizers/issues/321.
int res = REAL(fgetpwent_r)(fp, pwbuf, buf, buflen, pwbufp);
- if (!res) {
- if (pwbufp && *pwbufp) unpoison_passwd(ctx, *pwbufp);
- COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, buflen);
- }
+ if (!res && pwbufp && *pwbufp)
+ unpoison_passwd(ctx, *pwbufp);
if (pwbufp) COMMON_INTERCEPTOR_WRITE_RANGE(ctx, pwbufp, sizeof(*pwbufp));
return res;
}
@@ -2091,10 +2077,8 @@ INTERCEPTOR(int, fgetgrent_r, void *fp,
// its metadata. See
// https://github.com/google/sanitizers/issues/321.
int res = REAL(fgetgrent_r)(fp, pwbuf, buf, buflen, pwbufp);
- if (!res) {
- if (pwbufp && *pwbufp) unpoison_group(ctx, *pwbufp);
- COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, buflen);
- }
+ if (!res && pwbufp && *pwbufp)
+ unpoison_group(ctx, *pwbufp);
if (pwbufp) COMMON_INTERCEPTOR_WRITE_RANGE(ctx, pwbufp, sizeof(*pwbufp));
return res;
}
Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc?rev=353351&r1=353350&r2=353351&view=diff
==============================================================================
--- compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc (original)
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc Wed Feb 6 16:08:14 2019
@@ -2203,19 +2203,6 @@ static void HandleRecvmsg(ThreadState *t
#include "sanitizer_common/sanitizer_platform_interceptors.h"
// Causes interceptor recursion (getaddrinfo() and fopen())
#undef SANITIZER_INTERCEPT_GETADDRINFO
-// There interceptors do not seem to be strictly necessary for tsan.
-// But we see cases where the interceptors consume 70% of execution time.
-// Memory blocks passed to fgetgrent_r are "written to" by tsan several times.
-// First, there is some recursion (getgrnam_r calls fgetgrent_r), and each
-// function "writes to" the buffer. Then, the same memory is "written to"
-// twice, first as buf and then as pwbufp (both of them refer to the same
-// addresses).
-#undef SANITIZER_INTERCEPT_GETPWENT
-#undef SANITIZER_INTERCEPT_GETPWENT_R
-#undef SANITIZER_INTERCEPT_FGETPWENT
-#undef SANITIZER_INTERCEPT_FGETPWENT_R
-#undef SANITIZER_INTERCEPT_GETPWNAM_AND_FRIENDS
-#undef SANITIZER_INTERCEPT_GETPWNAM_R_AND_FRIENDS
// We define our own.
#if SANITIZER_INTERCEPT_TLS_GET_ADDR
#define NEED_TLS_GET_ADDR
Added: compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/getpw_getgr.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/getpw_getgr.cc?rev=353351&view=auto
==============================================================================
--- compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/getpw_getgr.cc (added)
+++ compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/getpw_getgr.cc Wed Feb 6 16:08:14 2019
@@ -0,0 +1,112 @@
+// RUN: %clangxx %s -o %t && %run %t
+// UNSUPPORTED: android, ios
+
+#include <assert.h>
+#include <grp.h>
+#include <memory>
+#include <pwd.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <string>
+
+std::string any_group;
+const int N = 123456;
+
+void Check(const char *str) {
+ assert(strlen(str) != N);
+}
+
+void Check(const passwd *result) {
+ Check(result->pw_name);
+ Check(result->pw_passwd);
+ assert(result->pw_uid != N);
+ assert(result->pw_gid != N);
+#if !defined(__ANDROID__)
+ Check(result->pw_gecos);
+#endif
+ Check(result->pw_dir);
+
+#if defined(__APPLE__) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__)
+ assert(result->pw_change != N);
+ Check(result->pw_class);
+ assert(result->pw_expire != N);
+#endif
+
+#if defined(__FreeBSD__)
+ assert(result->pw_fields != N);
+#endif
+
+ // SunOS also has pw_age and pw_comment which are documented as unused.
+}
+
+void Check(const group *result) {
+ Check(result->gr_name);
+ Check(result->gr_passwd);
+ assert(result->gr_gid != N);
+ for (char **mem = result->gr_mem; *mem; ++mem)
+ Check(*mem);
+ if (any_group.empty())
+ any_group = result->gr_name;
+}
+
+template <class T, class Fn, class... Args>
+void test(Fn f, Args... args) {
+ T *result = f(args...);
+ Check(result);
+}
+
+template <class T, class Fn, class... Args>
+void test_r(Fn f, Args... args) {
+ T gr;
+ T *result;
+ char buff[10000];
+ assert(!f(args..., &gr, buff, sizeof(buff), &result));
+ Check(&gr);
+ Check(result);
+}
+
+int main(int argc, const char *argv[]) {
+ test<passwd>(&getpwuid, 0);
+ test<passwd>(&getpwnam, "root");
+ test<group>(&getgrgid, 0);
+ test<group>(&getgrnam, any_group.c_str());
+
+#if !defined(__ANDROID__)
+ setpwent();
+ test<passwd>(&getpwent);
+ setgrent();
+ test<group>(&getgrent);
+
+#if !defined(__APPLE__)
+ setpwent();
+ test_r<passwd>(&getpwent_r);
+ setgrent();
+ test_r<group>(&getgrent_r);
+#endif
+
+ test_r<passwd>(&getpwuid_r, 0);
+ test_r<passwd>(&getpwnam_r, "root");
+
+ test_r<group>(&getgrgid_r, 0);
+ test_r<group>(&getgrnam_r, any_group.c_str());
+
+#if defined(__linux__)
+ auto pwd_file = [] {
+ return std::unique_ptr<FILE, decltype(&fclose)>(fopen("/etc/passwd", "r"),
+ &fclose);
+ };
+ auto gr_file = [] {
+ return std::unique_ptr<FILE, decltype(&fclose)>(fopen("/etc/group", "r"),
+ &fclose);
+ };
+ test<passwd>(&fgetpwent, pwd_file().get());
+ test<group>(&fgetgrent, gr_file().get());
+ test_r<passwd>(&fgetpwent_r, pwd_file().get());
+ test_r<group>(&fgetgrent_r, gr_file().get());
+#endif
+
+#endif // __ANDROID__
+
+ return 0;
+}
More information about the llvm-commits
mailing list