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

lib/sanitizer_common/sanitizer_common_interceptors.inc

INTERCEPTOR(int, sysinfo, void *info) {
   return res;
 }
 #define INIT_SYSINFO COMMON_INTERCEPT_FUNCTION(sysinfo);
 #else
 #define INIT_SYSINFO
 #endif
 
 #if SANITIZER_INTERCEPT_READDIR
+// On FreeBSD opendir() relies on strlen() so we have to intercept it.
+INTERCEPTOR(__sanitizer_dirent *, opendir, const char *filename) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, opendir, filename);
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, filename, REAL(strlen)(filename) + 1);
+  return REAL(opendir)(filename);
+}
+
 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) {