Add interceptors for readlinkat, name_to_handle_at, open_by_handle_at

[oliverchang]

Summary

Also move existing readlink msan interceptor to sanitizer_common.

Fixes google/sanitizers#908

[oliverchang]

minor nits

[oliverchang]

check unpoisoned

[oliverchang]

typo

[krytarowski]

lib/msan/msan_interceptors.cc

150 ↗

(On Diff #131740)

Can push this interceptor to the generic code for all sanitizers?

[eugenis]

lib/msan/msan_interceptors.cc

150 ↗

(On Diff #131740)

Ideally, both interceptors should be in sanitizer_common.

[vitalybuka]

lib/msan/msan_interceptors.cc

1050 ↗

(On Diff #131740)

Maybe intercept open_by_handle_at as well?

[oliverchang]

Move to sanitizer_common, add open_by_handle_at interceptor.

[oliverchang]

lib/msan/msan_interceptors.cc

150 ↗

(On Diff #131740)

Done, but I'm not sure where to move the tests (if I need to add all).

1050 ↗

(On Diff #131740)

Done, but not sure how to test it since it requires CAP_DAC_READ_SEARCH

lib/sanitizer_common/sanitizer_platform_interceptors.h

449 ↗

(On Diff #131897)

have a questions here: what happens when a function doesn't exist on a particular platform? it seems like readlinkat support was added fairly recently to certain platforms (e.g. added in MacOS 10.10) and may not be available everywhere despite it being part of posix.

[krytarowski]

lib/msan/msan_interceptors.cc

150 ↗

(On Diff #131740)

compiler-rt/test/sanitizer_common/

[oliverchang]

remove unneeded inclueds

[vitalybuka]

LGTM if you add sanitizer_common/ tests

lib/sanitizer_common/sanitizer_common_interceptors.inc

6641 ↗

(On Diff #131900)

SSIZE_T res = REAL(readlink)(path, buf, bufsiz);

test/msan/name_to_handle_at.cc

16 ↗

(On Diff #131900)

this test shouls stay here at it's msan specific

another test, with trivial function call, should be added under sanitizer_common/
sanitizer_common/ tests will be executed with other sanitizers

[oliverchang]

sanitizer_common tests, address comments

[oliverchang]

lib/sanitizer_common/sanitizer_common_interceptors.inc

6641 ↗

(On Diff #131900)

Done here and in other interceptors also.

test/msan/name_to_handle_at.cc

16 ↗

(On Diff #131900)

Done.

[oliverchang]

Thanks! Would you mind committing this for me?

[krytarowski]

lib/sanitizer_common/sanitizer_common_interceptors.inc

6947 ↗

(On Diff #131915)

I would drop empty newlines.

test/sanitizer_common/TestCases/Linux/readlink.cc

1 ↗

(On Diff #131915)

This test should be available for all POSIX systems.

It works on NetBSD as it is.

[krytarowski]

test/sanitizer_common/TestCases/Linux/readlink.cc

1 ↗

(On Diff #131915)

It might be a good idea to split the readlink(2) and readlinkat(2) into separate tests.
Also this -m64 should be dropped.

[vitalybuka]

test/sanitizer_common/TestCases/Linux/readlink.cc

1 ↗

(On Diff #131915)

sgtm
Kamil, would you like to do so and land the patch?
No problem If not. I will split myself. I just have nothing other than Linux.

[vitalybuka]

test/sanitizer_common/TestCases/Linux/readlink.cc

1 ↗

(On Diff #131915)

Don't bother, I am going to that myself.

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

if (res > 0)
     __msan_unpoison(ptr, res *size);
   return res;
 }
 #define MSAN_MAYBE_INTERCEPT_FREAD_UNLOCKED INTERCEPT_FUNCTION(fread_unlocked)
 #else
 #define MSAN_MAYBE_INTERCEPT_FREAD_UNLOCKED
 #endif
 
-INTERCEPTOR(SSIZE_T, readlink, const char *path, char *buf, SIZE_T bufsiz) {
-  ENSURE_MSAN_INITED();
-  CHECK_UNPOISONED_STRING(path, 0);
-  SSIZE_T res = REAL(readlink)(path, buf, bufsiz);
-  if (res > 0)
-    __msan_unpoison(buf, res);
-  return res;
-}
-
 #if !SANITIZER_NETBSD
 INTERCEPTOR(void *, mempcpy, void *dest, const void *src, SIZE_T n) {
   return (char *)__msan_memcpy(dest, src, n) + n;
 }
 #define MSAN_MAYBE_INTERCEPT_MEMPCPY INTERCEPT_FUNCTION(mempcpy)
 #else
 #define MSAN_MAYBE_INTERCEPT_MEMPCPY
 #endif
void InitializeInterceptors() {
   INTERCEPT_FUNCTION(free);
   MSAN_MAYBE_INTERCEPT_CFREE;
   MSAN_MAYBE_INTERCEPT_MALLOC_USABLE_SIZE;
   MSAN_MAYBE_INTERCEPT_MALLINFO;
   MSAN_MAYBE_INTERCEPT_MALLOPT;
   MSAN_MAYBE_INTERCEPT_MALLOC_STATS;
   INTERCEPT_FUNCTION(fread);
   MSAN_MAYBE_INTERCEPT_FREAD_UNLOCKED;
-  INTERCEPT_FUNCTION(readlink);
   INTERCEPT_FUNCTION(memccpy);
   MSAN_MAYBE_INTERCEPT_MEMPCPY;
   INTERCEPT_FUNCTION(bcopy);
   INTERCEPT_FUNCTION(wmemset);
   INTERCEPT_FUNCTION(wmemcpy);
   MSAN_MAYBE_INTERCEPT_WMEMPCPY;
   INTERCEPT_FUNCTION(wmemmove);
   INTERCEPT_FUNCTION(strcpy);  // NOLINT

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

 
 TEST(MemorySanitizer, readlink) {
   char *x = new char[1000];
   readlink(SYMLINK_TO_READ, x, 1000);
   EXPECT_NOT_POISONED(x[0]);
   delete [] x;
 }
 
+TEST(MemorySanitizer, readlinkat) {
+  char *x = new char[1000];
+  readlinkat(AT_FDCWD, SYMLINK_TO_READ, x, 1000);
+  EXPECT_NOT_POISONED(x[0]);
+  delete[] x;
+}
+
 TEST(MemorySanitizer, stat) {
   struct stat* st = new struct stat;
   int res = stat(FILE_TO_READ, st);
   ASSERT_EQ(0, res);
   EXPECT_NOT_POISONED(st->st_dev);
   EXPECT_NOT_POISONED(st->st_mode);
   EXPECT_NOT_POISONED(st->st_size);
 }

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

 
 #define INIT_GETGROUPLIST \
   COMMON_INTERCEPT_FUNCTION(getgrouplist); \
   COMMON_INTERCEPT_FUNCTION(getgroupmembership);
 #else
 #define INIT_GETGROUPLIST
 #endif
 
+#if SANITIZER_INTERCEPT_READLINK
+INTERCEPTOR(SSIZE_T, readlink, const char *path, char *buf, SIZE_T bufsiz) {
+  void* ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, readlink, path, buf, bufsiz);
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, path, REAL(strlen)(path) + 1);
+  SSIZE_T res = REAL(readlink)(path, buf, bufsiz);
+  if (res > 0)
+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, res);
+  return res;
+}
+
+#define INIT_READLINK COMMON_INTERCEPT_FUNCTION(readlink)
+#else
+#define INIT_READLINK
+#endif
+
+#if SANITIZER_INTERCEPT_READLINKAT
+INTERCEPTOR(SSIZE_T, readlinkat, int dirfd, const char *path, char *buf,
+            SIZE_T bufsiz) {
+  void* ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, readlinkat, dirfd, path, buf, bufsiz);
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, path, REAL(strlen)(path) + 1);
+  SSIZE_T res = REAL(readlinkat)(dirfd, path, buf, bufsiz);
+  if (res > 0)
+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, buf, res);
+  return res;
+}
+
+#define INIT_READLINKAT COMMON_INTERCEPT_FUNCTION(readlinkat)
+#else
+#define INIT_READLINKAT
+#endif
+
+#if SANITIZER_INTERCEPT_NAME_TO_HANDLE_AT
+INTERCEPTOR(int, name_to_handle_at, int dirfd, const char *pathname,
+            struct file_handle *handle, int *mount_id, int flags) {
+  void* ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, name_to_handle_at, dirfd, pathname, handle,
+                           mount_id, flags);
+  COMMON_INTERCEPTOR_READ_RANGE(ctx, pathname, REAL(strlen)(pathname) + 1);
+
+  __sanitizer_file_handle *sanitizer_handle =
+      reinterpret_cast<__sanitizer_file_handle*>(handle);
+  COMMON_INTERCEPTOR_READ_RANGE(
+      ctx, &sanitizer_handle->handle_bytes,
+      sizeof(sanitizer_handle->handle_bytes));
+
+  int res = REAL(name_to_handle_at)(dirfd, pathname, handle, mount_id, flags);
+  if (!res) {
+    COMMON_INTERCEPTOR_WRITE_RANGE(
+        ctx, &sanitizer_handle->handle_bytes,
+        sizeof(sanitizer_handle->handle_bytes));
+    COMMON_INTERCEPTOR_WRITE_RANGE(
+        ctx, &sanitizer_handle->handle_type,
+        sizeof(sanitizer_handle->handle_type));
+    COMMON_INTERCEPTOR_WRITE_RANGE(
+        ctx, &sanitizer_handle->f_handle, sanitizer_handle->handle_bytes);
+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, mount_id, sizeof(*mount_id));
+  }
+  return res;
+}
+
+#define INIT_NAME_TO_HANDLE_AT COMMON_INTERCEPT_FUNCTION(name_to_handle_at)
+#else
+#define INIT_NAME_TO_HANDLE_AT
+#endif
+
+#if SANITIZER_INTERCEPT_OPEN_BY_HANDLE_AT
+INTERCEPTOR(int, open_by_handle_at, int mount_fd, struct file_handle* handle,
+            int flags) {
+  void* ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, open_by_handle_at, mount_fd, handle, flags);
+
+  __sanitizer_file_handle *sanitizer_handle =
+      reinterpret_cast<__sanitizer_file_handle*>(handle);
+  COMMON_INTERCEPTOR_READ_RANGE(
+      ctx, &sanitizer_handle->handle_bytes,
+      sizeof(sanitizer_handle->handle_bytes));
+  COMMON_INTERCEPTOR_READ_RANGE(
+      ctx, &sanitizer_handle->handle_type,
+      sizeof(sanitizer_handle->handle_type));
+  COMMON_INTERCEPTOR_READ_RANGE(
+      ctx, &sanitizer_handle->f_handle, sanitizer_handle->handle_bytes);
+
+  return REAL(open_by_handle_at)(mount_fd, handle, flags);
+}
+
+#define INIT_OPEN_BY_HANDLE_AT COMMON_INTERCEPT_FUNCTION(open_by_handle_at)
+#else
+#define INIT_OPEN_BY_HANDLE_AT
+#endif
+
 static void InitializeCommonInterceptors() {
   static u64 metadata_mem[sizeof(MetadataHashMap) / sizeof(u64) + 1];
   interceptor_metadata_map = new((void *)&metadata_mem) MetadataHashMap();
 
   INIT_TEXTDOMAIN;
   INIT_STRLEN;
   INIT_STRNLEN;
   INIT_STRNDUP;
static void InitializeCommonInterceptors() {
   INIT_ACCT;
   INIT_USER_FROM_UID;
   INIT_UID_FROM_USER;
   INIT_GROUP_FROM_GID;
   INIT_GID_FROM_GROUP;
   INIT_ACCESS;
   INIT_FACCESSAT;
   INIT_GETGROUPLIST;
+  INIT_READLINK;
+  INIT_READLINKAT;
+
+  INIT_NAME_TO_HANDLE_AT;
+  INIT_OPEN_BY_HANDLE_AT;
 
 #if SANITIZER_NETBSD
   COMMON_INTERCEPT_FUNCTION(__libc_mutex_lock);
   COMMON_INTERCEPT_FUNCTION(__libc_mutex_unlock);
   COMMON_INTERCEPT_FUNCTION(__libc_thr_setcancelstate);
 #endif
 
   INIT___PRINTF_CHK;
 }

compiler-rt/trunk/lib/sanitizer_common/sanitizer_platform_interceptors.h

 #define SANITIZER_INTERCEPT_USER_FROM_UID SI_NETBSD
 #define SANITIZER_INTERCEPT_UID_FROM_USER SI_NETBSD
 #define SANITIZER_INTERCEPT_GROUP_FROM_GID SI_NETBSD
 #define SANITIZER_INTERCEPT_GID_FROM_GROUP SI_NETBSD
 #define SANITIZER_INTERCEPT_ACCESS SI_NETBSD
 #define SANITIZER_INTERCEPT_FACCESSAT SI_NETBSD
 #define SANITIZER_INTERCEPT_GETGROUPLIST SI_NETBSD
 
+#define SANITIZER_INTERCEPT_NAME_TO_HANDLE_AT SI_LINUX
+#define SANITIZER_INTERCEPT_OPEN_BY_HANDLE_AT SI_LINUX
+
+#define SANITIZER_INTERCEPT_READLINK SI_POSIX
+#define SANITIZER_INTERCEPT_READLINKAT SI_POSIX
+
 #endif  // #ifndef SANITIZER_PLATFORM_INTERCEPTORS_H

compiler-rt/trunk/lib/sanitizer_common/sanitizer_platform_limits_posix.h

#if SANITIZER_LINUX
   struct __sanitizer_mntent {
     char *mnt_fsname;
     char *mnt_dir;
     char *mnt_type;
     char *mnt_opts;
     int mnt_freq;
     int mnt_passno;
   };
+
+  struct __sanitizer_file_handle {
+    unsigned int handle_bytes;
+    int handle_type;
+    unsigned char f_handle[1];  // variable sized
+  };
 #endif
 
 #if SANITIZER_MAC || SANITIZER_FREEBSD
   struct __sanitizer_msghdr {
     void *msg_name;
     unsigned msg_namelen;
     struct __sanitizer_iovec *msg_iov;
     unsigned msg_iovlen;

compiler-rt/trunk/test/msan/Linux/name_to_handle_at.cc

+// RUN: %clangxx_msan -std=c++11 -O0 -g %s -o %t && %run %t
+
+#include <assert.h>
+#include <fcntl.h>
+#include <sanitizer/msan_interface.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+int main(void) {
+  struct file_handle *handle = reinterpret_cast<struct file_handle *>(
+      malloc(sizeof(*handle) + MAX_HANDLE_SZ));
+  handle->handle_bytes = MAX_HANDLE_SZ;
+
+  int mount_id;
+  int res = name_to_handle_at(AT_FDCWD, "/bin/cat", handle, &mount_id, 0);
+  assert(!res);
+  __msan_check_mem_is_initialized(&mount_id, sizeof(mount_id));
+  __msan_check_mem_is_initialized(&handle->handle_bytes,
+                                  sizeof(handle->handle_bytes));
+  __msan_check_mem_is_initialized(&handle->handle_type,
+                                  sizeof(handle->handle_type));
+  __msan_check_mem_is_initialized(&handle->f_handle, handle->handle_bytes);
+
+  free(handle);
+  return 0;
+}

compiler-rt/trunk/test/sanitizer_common/TestCases/Linux/name_to_handle_at.cc

+// RUN: %clangxx -O0 %s -o %t && %run %t
+
+#include <assert.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
+int main(int argc, char **argv) {
+  int mount_id;
+  struct file_handle *handle = reinterpret_cast<struct file_handle *>(
+      malloc(sizeof(*handle) + MAX_HANDLE_SZ));
+
+  handle->handle_bytes = MAX_HANDLE_SZ;
+  int res = name_to_handle_at(AT_FDCWD, argv[0], handle, &mount_id, 0);
+  assert(!res);
+
+  free(handle);
+  return 0;
+}

compiler-rt/trunk/test/sanitizer_common/TestCases/Linux/readlinkat.c

+// RUN: %clang -O0 %s -o %t && %run %t
+
+#include <assert.h>
+#include <fcntl.h>
+#include <linux/limits.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+int main(int argc, char **argv) {
+  char symlink_path[PATH_MAX];
+  snprintf(symlink_path, sizeof(symlink_path), "%s_%d.symlink", argv[0],
+           getpid());
+  int res = symlink(argv[0], symlink_path);
+  assert(!res);
+
+  char readlinkat_path[PATH_MAX];
+  int res2 = readlinkat(AT_FDCWD, symlink_path, readlinkat_path,
+                        sizeof(readlinkat_path));
+  assert(res2 >= 0);
+  readlinkat_path[res2] = '\0';
+  assert(!strcmp(readlinkat_path, argv[0]));
+
+  return 0;
+}

compiler-rt/trunk/test/sanitizer_common/TestCases/Posix/readlink.c

+// RUN: %clang -O0 %s -o %t && %run %t
+
+#include <assert.h>
+#include <fcntl.h>
+#include <linux/limits.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+int main(int argc, char **argv) {
+  char symlink_path[PATH_MAX];
+  snprintf(symlink_path, sizeof(symlink_path), "%s_%d.symlink", argv[0],
+           getpid());
+  int res = symlink(argv[0], symlink_path);
+  assert(!res);
+
+  char readlink_path[PATH_MAX];
+  ssize_t res2 = readlink(symlink_path, readlink_path, sizeof(readlink_path));
+  assert(res2 >= 0);
+  readlink_path[res2] = '\0';
+  assert(!strcmp(readlink_path, argv[0]));
+
+  char readlinkat_path[PATH_MAX];
+  res2 = readlinkat(AT_FDCWD, symlink_path, readlinkat_path,
+                    sizeof(readlink_path));
+  assert(res2 >= 0);
+  readlinkat_path[res2] = '\0';
+  assert(!strcmp(readlinkat_path, argv[0]));
+
+  return 0;
+}