Skip to content

Conversation

@cjappl
Copy link
Contributor

@cjappl cjappl commented Nov 21, 2024

No description provided.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drawing folks attention here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here

@llvmbot
Copy link
Member

llvmbot commented Nov 21, 2024

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

Author: Chris Apple (cjappl)

Changes

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

2 Files Affected:

  • (modified) compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp (+22)
  • (added) compiler-rt/test/rtsan/fork_exec.cpp (+61)
diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
index 91d023e858ba3b..5296dd8039c9f1 100644
--- a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
@@ -20,6 +20,8 @@
 #include "interception/interception.h"
 #include "rtsan/rtsan.h"
 
+#include <cassert>
+
 #if SANITIZER_APPLE
 
 #if TARGET_OS_MAC
@@ -47,6 +49,7 @@ void OSSpinLockLock(volatile OSSpinLock *__lock);
 #include <stdarg.h>
 #include <stdio.h>
 #include <sys/socket.h>
+#include <sys/wait.h>
 #include <time.h>
 #include <unistd.h>
 
@@ -751,6 +754,22 @@ INTERCEPTOR(int, mkfifo, const char *pathname, mode_t mode) {
   return REAL(mkfifo)(pathname, mode);
 }
 
+INTERCEPTOR(pid_t, fork, void) {
+  __rtsan_notify_intercepted_call("fork");
+  return REAL(fork)();
+}
+
+INTERCEPTOR(int, execve, const char *filename, char *const argv[],
+            char *const envp[]) {
+  __rtsan_notify_intercepted_call("execve");
+  return REAL(execve)(filename, argv, envp);
+}
+
+// TODO: the `wait` family of functions is an oddity. In testing, if you
+// intercept them, Darwin seemingly ignores them, and linux never returns from
+// the test. Revisit this in the future, but hopefully intercepting fork/exec is
+// enough to dissuade usage of wait by proxy.
+
 // Preinit
 void __rtsan::InitializeInterceptors() {
   INTERCEPT_FUNCTION(calloc);
@@ -855,6 +874,9 @@ void __rtsan::InitializeInterceptors() {
 
   INTERCEPT_FUNCTION(pipe);
   INTERCEPT_FUNCTION(mkfifo);
+
+  INTERCEPT_FUNCTION(fork);
+  INTERCEPT_FUNCTION(execve);
 }
 
 #endif // SANITIZER_POSIX
diff --git a/compiler-rt/test/rtsan/fork_exec.cpp b/compiler-rt/test/rtsan/fork_exec.cpp
new file mode 100644
index 00000000000000..23fddcac5a24de
--- /dev/null
+++ b/compiler-rt/test/rtsan/fork_exec.cpp
@@ -0,0 +1,61 @@
+// RUN: %clangxx -fsanitize=realtime -DIS_NONBLOCKING=1 %s -o %t
+// RUN: %env_rtsan_opts="halt_on_error=true" not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-HALT
+// RUN: %env_rtsan_opts="halt_on_error=false" %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-NOHALT
+
+// RUN: %clangxx -fsanitize=realtime -DIS_NONBLOCKING=0 %s -o %t
+// RUN: %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-OK
+// RUN: %env_rtsan_opts="halt_on_error=false" %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-OK
+
+// UNSUPPORTED: ios
+
+// Intent: Ensure fork/exec dies when realtime and survives otherwise
+//         This behavior is difficult to test in a gtest, because the process is
+//         wiped away with exec.
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#if IS_NONBLOCKING
+#  define MAYBE_NONBLOCKING [[clang::nonblocking]]
+#else
+#  define MAYBE_NONBLOCKING
+#endif
+
+#include <cassert>
+
+int main() MAYBE_NONBLOCKING {
+  const pid_t pid = fork();
+
+  if (pid == 0) {
+    char *args[] = {"/bin/ls", nullptr};
+    execve(args[0], args, nullptr);
+    perror("execve failed");
+    return 1;
+  } else if (pid > 0) {
+    int status;
+    waitpid(pid, &status, 0);
+    usleep(1);
+  } else {
+    perror("fork failed");
+    return 1;
+  }
+
+  printf("fork/exec succeeded\n");
+  return 0;
+}
+
+// CHECK-NOHALT: Intercepted call to {{.*}} `fork` {{.*}}
+// CHECK-NOHALT: Intercepted call to {{.*}} `execve` {{.*}}
+
+// usleep checks that rtsan is still enabled in the parent process
+// See note in our interceptors file for why we don't look for `wait`
+// CHECK-NOHALT: Intercepted call to {{.*}} `usleep` {{.*}}
+
+// CHECK-NOHALT: fork/exec succeeded
+
+// CHECK-HALT: ==ERROR: RealtimeSanitizer: unsafe-library-call
+// CHECK-HALT-NEXT: Intercepted call to {{.*}} `fork` {{.*}}
+
+// CHECK-OK: fork/exec succeeded

@cjappl
Copy link
Contributor Author

cjappl commented Dec 2, 2024

Ping reviewers @davidtrevelyan

@cjappl cjappl merged commit af4ae12 into llvm:main Dec 5, 2024
7 checks passed
@cjappl cjappl deleted the fork_exec branch December 5, 2024 00:38
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.

3 participants