Skip to content

Conversation

@cjappl
Copy link
Contributor

@cjappl cjappl commented Dec 2, 2024

This is a complex one - syscall is used when people want to bypass libc and make the call directly

However, this call:

I've tried to put in a couple tests to ensure we aren't mucking with the underlying functionality and they seem to be working.

@graphite-app
Copy link

graphite-app bot commented Dec 2, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “FP Bundles” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@llvmbot
Copy link
Member

llvmbot commented Dec 2, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Chris Apple (cjappl)

Changes

This is a complex one - syscall is used when people want to bypass libc and make the call directly

However, this call:

I've tried to put in a couple tests to ensure we aren't mucking with the underlying functionality and they seem to be working.


Full diff: https://github.com/llvm/llvm-project/pull/118250.diff

3 Files Affected:

  • (modified) compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp (+39)
  • (modified) compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp (+17)
  • (added) compiler-rt/test/rtsan/syscall.cpp (+78)
diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
index 7dddaf553dad6d..4262039e8e1fa6 100644
--- a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
@@ -815,6 +815,43 @@ INTERCEPTOR(int, mkfifo, const char *pathname, mode_t mode) {
   return REAL(mkfifo)(pathname, mode);
 }
 
+#if SANITIZER_APPLE
+#define INT_TYPE_SYSCALL int
+#else
+#define INT_TYPE_SYSCALL long
+#endif
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+INTERCEPTOR(INT_TYPE_SYSCALL, syscall, INT_TYPE_SYSCALL number, ...) {
+  __rtsan_notify_intercepted_call("syscall");
+
+  va_list args;
+  va_start(args, number);
+
+  // the goal is to pick something large enough to hold all syscall args
+  // see fcntl for more discussion and why we always pull all 6 args
+  using arg_type = unsigned long;
+  arg_type arg1 = va_arg(args, arg_type);
+  arg_type arg2 = va_arg(args, arg_type);
+  arg_type arg3 = va_arg(args, arg_type);
+  arg_type arg4 = va_arg(args, arg_type);
+  arg_type arg5 = va_arg(args, arg_type);
+  arg_type arg6 = va_arg(args, arg_type);
+
+  // these are various examples of things that COULD be passed
+  static_assert(sizeof(arg_type) >= sizeof(off_t));
+  static_assert(sizeof(arg_type) >= sizeof(struct flock *));
+  static_assert(sizeof(arg_type) >= sizeof(const char *));
+  static_assert(sizeof(arg_type) >= sizeof(int));
+  static_assert(sizeof(arg_type) >= sizeof(unsigned long));
+
+  va_end(args);
+
+  return REAL(syscall)(number, arg1, arg2, arg3, arg4, arg5, arg6);
+}
+#pragma clang diagnostic pop
+
 // Preinit
 void __rtsan::InitializeInterceptors() {
   INTERCEPT_FUNCTION(calloc);
@@ -918,6 +955,8 @@ void __rtsan::InitializeInterceptors() {
 
   INTERCEPT_FUNCTION(pipe);
   INTERCEPT_FUNCTION(mkfifo);
+
+  INTERCEPT_FUNCTION(syscall);
 }
 
 #endif // SANITIZER_POSIX
diff --git a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp
index f715b0b11b2885..88df8ec46d0a33 100644
--- a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp
+++ b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp
@@ -47,6 +47,7 @@
 #include <sys/mman.h>
 #include <sys/socket.h>
 #include <sys/stat.h>
+#include <sys/syscall.h>
 #include <sys/types.h>
 #include <sys/uio.h>
 
@@ -1090,4 +1091,20 @@ TEST(TestRtsanInterceptors, PipeDiesWhenRealtime) {
   ExpectNonRealtimeSurvival(Func);
 }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+TEST(TestRtsanInterceptors, SyscallDiesWhenRealtime) {
+  auto Func = []() { syscall(SYS_getpid); };
+  ExpectRealtimeDeath(Func, "syscall");
+  ExpectNonRealtimeSurvival(Func);
+}
+
+TEST(TestRtsanInterceptors, GetPidReturnsSame) {
+  int pid = syscall(SYS_getpid);
+  EXPECT_THAT(pid, Ne(-1));
+
+  EXPECT_THAT(getpid(), Eq(pid));
+}
+#pragma clang diagnostic pop
+
 #endif // SANITIZER_POSIX
diff --git a/compiler-rt/test/rtsan/syscall.cpp b/compiler-rt/test/rtsan/syscall.cpp
new file mode 100644
index 00000000000000..a23a934184a691
--- /dev/null
+++ b/compiler-rt/test/rtsan/syscall.cpp
@@ -0,0 +1,78 @@
+// RUN: %clangxx -fsanitize=realtime %s -o %t
+// RUN: %env_rtsan_opts="halt_on_error=false" %run %t 2>&1 | FileCheck %s --check-prefixes=CHECK-RTSAN,CHECK
+
+// RUN: %clangxx %s -o %t
+// RUN: %run %t 2>&1 | FileCheck %s
+
+// UNSUPPORTED: ios
+
+// Intent: Ensure the `syscall` call behaves in the same way with/without the
+//         sanitizer disabled
+
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+const char *GetTemporaryFilePath() { return "/tmp/rtsan_syscall_test.txt"; }
+
+void custom_assert(bool condition, const char *message) {
+  if (!condition) {
+    fprintf(stderr, "ASSERTION FAILED: %s\n", message);
+    exit(1);
+  }
+}
+
+class ScopedFileCleanup {
+public:
+  [[nodiscard]] ScopedFileCleanup() = default;
+  ~ScopedFileCleanup() {
+    if (access(GetTemporaryFilePath(), F_OK) != -1)
+      unlink(GetTemporaryFilePath());
+  }
+};
+
+// Apple has deprecated `syscall`, ignore that error
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+int main() [[clang::nonblocking]] {
+  ScopedFileCleanup cleanup;
+
+  {
+    int fd = syscall(SYS_openat, AT_FDCWD, GetTemporaryFilePath(),
+                     O_CREAT | O_WRONLY, 0644);
+
+    custom_assert(fd != -1, "Failed to open file - write");
+
+    int written = syscall(SYS_write, fd, "Hello, world!", 13);
+    custom_assert(written == 13, "Failed to write to file");
+
+    custom_assert(syscall(SYS_close, fd) == 0, "Failed to close file - write");
+  }
+
+  {
+    int fd = syscall(SYS_openat, AT_FDCWD, GetTemporaryFilePath(), O_RDONLY);
+    custom_assert(fd != -1, "Failed to open file - read");
+
+    char buffer[13];
+    int read = syscall(SYS_read, fd, buffer, 13);
+    custom_assert(read == 13, "Failed to read from file");
+
+    custom_assert(memcmp(buffer, "Hello, world!", 13) == 0,
+                  "Read data does not match written data");
+
+    custom_assert(syscall(SYS_close, fd) == 0, "Failed to close file - read");
+  }
+
+  unlink(GetTemporaryFilePath());
+  printf("DONE\n");
+}
+#pragma clang diagnostic pop
+
+// CHECK-NOT: ASSERTION FAILED
+// CHECK-RTSAN-COUNT-6: Intercepted call to real-time unsafe function `syscall`
+
+// CHECK: DONE

@cjappl cjappl merged commit eae30a2 into llvm:main Dec 2, 2024
10 checks passed
@cjappl cjappl deleted the syscall branch December 2, 2024 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants