[Sanitizers] Intercept opendir()

[kcc]

Not sure I understand this.
If opendir() calls strlen(), which we already intersept, won't we see the access w/o this interceptor?

[ygribov]

Not sure I understand this.
If opendir() calls strlen(), which we already intersept, won't we see the access w/o this interceptor?

I thought that glibc usually forces references to it's own functions to be linked internally (to avoid PLT costs). So ASan interceptor may not be called.

[eugenis]

So the difference in observable behavior is we start detecting uninit in the opendir() argument? Why is this freebsd-specific?

Also, there should be a test.

[kutuzov.viktor.84]

If opendir() calls strlen(), which we already intersept, won't we see the access w/o this interceptor?

The point is that FreeBSD's opendir() calls strlen() for inspecting related internal structures and not the file path passed (the comment should mention it; my mistake). Then given we don't intercept opendir(), we indeed do catch poisoned file paths, but we also generate false positives on these internal structures. Since COMMON_INTERCEPTOR_ENTER() includes the declaration for interceptor scope, it suppresses the checks for whatever the real opendir() relies on.

[kutuzov.viktor.84]

• The opendir() interceptor moved to Msan so it doesn't interfere with the Tsan's one;
• The comment fixed to mention internal structures that cause false positives;
• A test added.

[eugenis]

Got it.
This interceptor is beneficial on linux, too - it adds check for opendir() path.
Also, we've been moving interceptors to sanitizer_common, not the other way around. Please move it back, and merge the TSan bits into it (see COMMON_INTERCEPTOR_FD_ACQUIRE).

[kutuzov.viktor.84]

Updated.

[eugenis]

Looks ok, but I dislike the use of Dir2addr() in common interceptors, which is only valid in TSan and looks completely out of context. Maybe wrap it into COMMON_INTERCEPTOR_ACQUIRE_DIR?
Dmitry?

[dvyukov]

yes

[kutuzov.viktor.84]

Updated.

[eugenis]

Perhaps COMMON_INTERCEPTOR_DIR_ACQUIRE, for similarity with FD_ACQUIRE / FD_RELEASE and MUTEX_LOCK / MUTEX_UNLOCK.

compiler-rt/trunk/lib/asan/asan_interceptors.cc

#define COMMON_INTERCEPTOR_ENTER(ctx, func, ...)                               \
   do {                                                                         \
     if (asan_init_is_running)                                                  \
       return REAL(func)(__VA_ARGS__);                                          \
     ASAN_INTERCEPTOR_ENTER(ctx, func);                                         \
     if (SANITIZER_MAC && UNLIKELY(!asan_inited))                               \
       return REAL(func)(__VA_ARGS__);                                          \
     ENSURE_ASAN_INITED();                                                      \
   } while (false)
+#define COMMON_INTERCEPTOR_DIR_ACQUIRE(ctx, path) \
+  do {                                            \
+  } while (false)
 #define COMMON_INTERCEPTOR_FD_ACQUIRE(ctx, fd) \
   do {                                         \
   } while (false)
 #define COMMON_INTERCEPTOR_FD_RELEASE(ctx, fd) \
   do {                                         \
   } while (false)
 #define COMMON_INTERCEPTOR_FD_SOCKET_ACCEPT(ctx, fd, newfd) \
   do {                                                      \

compiler-rt/trunk/lib/msan/msan_interceptors.cc

 #define COMMON_INTERCEPTOR_ENTER(ctx, func, ...)                  \
   if (msan_init_is_running) return REAL(func)(__VA_ARGS__);       \
   MSanInterceptorContext msan_ctx = {IsInInterceptorScope()};     \
   ctx = (void *)&msan_ctx;                                        \
   (void)ctx;                                                      \
   InterceptorScope interceptor_scope;                             \
   __msan_unpoison(__errno_location(), sizeof(int)); /* NOLINT */  \
   ENSURE_MSAN_INITED();
+#define COMMON_INTERCEPTOR_DIR_ACQUIRE(ctx, path) \
+  do {                                            \
+  } while (false)
 #define COMMON_INTERCEPTOR_FD_ACQUIRE(ctx, fd) \
   do {                                         \
   } while (false)
 #define COMMON_INTERCEPTOR_FD_RELEASE(ctx, fd) \
   do {                                         \
   } while (false)
 #define COMMON_INTERCEPTOR_FD_SOCKET_ACCEPT(ctx, fd, newfd) \
   do {                                                      \

compiler-rt/trunk/lib/msan/tests/msan_test.cc

TEST(MemorySanitizer, confstr) {
   res = confstr(_CS_PATH, buf2, sizeof(buf2));
   ASSERT_LT(res, sizeof(buf2));
   EXPECT_NOT_POISONED(buf2[0]);
   EXPECT_NOT_POISONED(buf2[res - 1]);
   EXPECT_POISONED(buf2[res]);
   ASSERT_EQ(res, strlen(buf2) + 1);
 }
 
+TEST(MemorySanitizer, opendir) {
+  DIR *dir = opendir(".");
+  closedir(dir);
+
+  char name[10] = ".";
+  __msan_poison(name, sizeof(name));
+  EXPECT_UMR(dir = opendir(name));
+  closedir(dir);
+}
+
 TEST(MemorySanitizer, readdir) {
   DIR *dir = opendir(".");
   struct dirent *d = readdir(dir);
   ASSERT_TRUE(d != NULL);
   EXPECT_NOT_POISONED(d->d_name[0]);
   closedir(dir);
 }
 

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc

 //
 // This file should be included into the tool's interceptor file,
 // which has to define it's own macros:
 //   COMMON_INTERCEPTOR_ENTER
 //   COMMON_INTERCEPTOR_ENTER_NOIGNORE
 //   COMMON_INTERCEPTOR_READ_RANGE
 //   COMMON_INTERCEPTOR_WRITE_RANGE
 //   COMMON_INTERCEPTOR_INITIALIZE_RANGE
+//   COMMON_INTERCEPTOR_DIR_ACQUIRE
 //   COMMON_INTERCEPTOR_FD_ACQUIRE
 //   COMMON_INTERCEPTOR_FD_RELEASE
 //   COMMON_INTERCEPTOR_FD_ACCESS
 //   COMMON_INTERCEPTOR_SET_THREAD_NAME
 //   COMMON_INTERCEPTOR_ON_EXIT
 //   COMMON_INTERCEPTOR_MUTEX_LOCK
 //   COMMON_INTERCEPTOR_MUTEX_UNLOCK
 //   COMMON_INTERCEPTOR_MUTEX_REPAIR
INTERCEPTOR(int, sysinfo, void *info) {
   return res;
 }
 #define INIT_SYSINFO COMMON_INTERCEPT_FUNCTION(sysinfo);
 #else
 #define INIT_SYSINFO
 #endif
 
 #if SANITIZER_INTERCEPT_READDIR
+INTERCEPTOR(__sanitizer_dirent *, opendir, const char *path) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, opendir, path);
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, path, REAL(strlen)(path) + 1);
+  __sanitizer_dirent *res = REAL(opendir)(path);
+  if (res != 0)
+    COMMON_INTERCEPTOR_DIR_ACQUIRE(ctx, path);
+  return res;
+}
+
 INTERCEPTOR(__sanitizer_dirent *, readdir, void *dirp) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, readdir, dirp);
   // FIXME: under ASan the call below may write to freed memory and corrupt
   // its metadata. See
   // https://code.google.com/p/address-sanitizer/issues/detail?id=321.
   __sanitizer_dirent *res = REAL(readdir)(dirp);
   if (res) COMMON_INTERCEPTOR_WRITE_RANGE(ctx, res, res->d_reclen);
if (!res) {
     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, result, sizeof(*result));
     if (*result)
       COMMON_INTERCEPTOR_WRITE_RANGE(ctx, *result, (*result)->d_reclen);
   }
   return res;
 }
 
 #define INIT_READDIR                  \
+  COMMON_INTERCEPT_FUNCTION(opendir); \
   COMMON_INTERCEPT_FUNCTION(readdir); \
   COMMON_INTERCEPT_FUNCTION(readdir_r);
 #else
 #define INIT_READDIR
 #endif
 
 #if SANITIZER_INTERCEPT_READDIR64
 INTERCEPTOR(__sanitizer_dirent64 *, readdir64, void *dirp) {

compiler-rt/trunk/lib/tsan/rtl/tsan_interceptors.cc

 
 TSAN_INTERCEPTOR(int, rmdir, char *path) {
   SCOPED_TSAN_INTERCEPTOR(rmdir, path);
   Release(thr, pc, Dir2addr(path));
   int res = REAL(rmdir)(path);
   return res;
 }
 
-TSAN_INTERCEPTOR(void*, opendir, char *path) {
-  SCOPED_TSAN_INTERCEPTOR(opendir, path);
-  void *res = REAL(opendir)(path);
-  if (res != 0)
-    Acquire(thr, pc, Dir2addr(path));
-  return res;
-}
-
 TSAN_INTERCEPTOR(int, closedir, void *dirp) {
   SCOPED_TSAN_INTERCEPTOR(closedir, dirp);
   int fd = dirfd(dirp);
   FdClose(thr, pc, fd);
   return REAL(closedir)(dirp);
 }
 
 #if !SANITIZER_FREEBSD
#define COMMON_INTERCEPTOR_FILE_CLOSE(ctx, file) \
   }
 
 #define COMMON_INTERCEPTOR_LIBRARY_LOADED(filename, res)  \
   libignore()->OnLibraryLoaded(filename)
 
 #define COMMON_INTERCEPTOR_LIBRARY_UNLOADED() \
   libignore()->OnLibraryUnloaded()
 
+#define COMMON_INTERCEPTOR_DIR_ACQUIRE(ctx, path) \
+  Acquire(((TsanInterceptorContext *) ctx)->thr, pc, Dir2addr(path))
+
 #define COMMON_INTERCEPTOR_FD_ACQUIRE(ctx, fd) \
   FdAcquire(((TsanInterceptorContext *) ctx)->thr, pc, fd)
 
 #define COMMON_INTERCEPTOR_FD_RELEASE(ctx, fd) \
   FdRelease(((TsanInterceptorContext *) ctx)->thr, pc, fd)
 
 #define COMMON_INTERCEPTOR_FD_ACCESS(ctx, fd) \
   FdAccess(((TsanInterceptorContext *) ctx)->thr, pc, fd)
#endif
   TSAN_INTERCEPT(unlink);
   TSAN_INTERCEPT(tmpfile);
   TSAN_MAYBE_INTERCEPT_TMPFILE64;
   TSAN_INTERCEPT(fread);
   TSAN_INTERCEPT(fwrite);
   TSAN_INTERCEPT(abort);
   TSAN_INTERCEPT(puts);
   TSAN_INTERCEPT(rmdir);
-  TSAN_INTERCEPT(opendir);
   TSAN_INTERCEPT(closedir);
 
   TSAN_MAYBE_INTERCEPT_EPOLL_CTL;
   TSAN_MAYBE_INTERCEPT_EPOLL_WAIT;
 
   TSAN_INTERCEPT(sigaction);
   TSAN_INTERCEPT(signal);
   TSAN_INTERCEPT(sigsuspend);