Skip to content

[rtsan] Add pipe, mkfifo interceptors #116915

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 20, 2024
Merged

[rtsan] Add pipe, mkfifo interceptors #116915

merged 1 commit into from
Nov 20, 2024

Conversation

cjappl
Copy link
Contributor

@cjappl cjappl commented Nov 20, 2024

Why we think this are unsafe

Again, these correspond directly to system calls on linux and OSX. They are two ways to do interprocess communication so it would make sense that they take some synchronization by the OS.

@llvmbot
Copy link
Member

llvmbot commented Nov 20, 2024

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

Author: Chris Apple (cjappl)

Changes

Why we think this are unsafe

Again, these correspond directly to system calls on linux and OSX. They are two ways to do interprocess communication so it would make sense that they take some synchronization by the OS.


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

2 Files Affected:

  • (modified) compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp (+24)
  • (modified) compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp (+28)
diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
index d18ebb57fc2f75..5ef52d72fceb31 100644
--- a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
@@ -736,6 +736,26 @@ INTERCEPTOR(int, kevent64, int kq, const struct kevent64_s *changelist,
 #define RTSAN_MAYBE_INTERCEPT_KEVENT64
 #endif // SANITIZER_INTERCEPT_KQUEUE
 
+INTERCEPTOR(int, pipe, int pipefd[2]) {
+  __rtsan_notify_intercepted_call("pipe");
+  return REAL(pipe)(pipefd);
+}
+
+INTERCEPTOR(int, mkfifo, const char *pathname, mode_t mode) {
+  __rtsan_notify_intercepted_call("mkfifo");
+  return REAL(mkfifo)(pathname, mode);
+}
+
+// see comment above about -Wunguarded-availability-new
+// and why we disable it here
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunguarded-availability-new"
+INTERCEPTOR(int, mkfifoat, int dirfd, const char *pathname, mode_t mode) {
+  __rtsan_notify_intercepted_call("mkfifoat");
+  return REAL(mkfifoat)(dirfd, pathname, mode);
+}
+#pragma clang diagnostic pop
+
 // Preinit
 void __rtsan::InitializeInterceptors() {
   INTERCEPT_FUNCTION(calloc);
@@ -836,6 +856,10 @@ void __rtsan::InitializeInterceptors() {
   RTSAN_MAYBE_INTERCEPT_KQUEUE;
   RTSAN_MAYBE_INTERCEPT_KEVENT;
   RTSAN_MAYBE_INTERCEPT_KEVENT64;
+
+  INTERCEPT_FUNCTION(pipe);
+  INTERCEPT_FUNCTION(mkfifo);
+  INTERCEPT_FUNCTION(mkfifoat);
 }
 
 #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 ab02b44187d89e..72f6d819b216c0 100644
--- a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp
+++ b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp
@@ -965,4 +965,32 @@ TEST_F(KqueueTest, Kevent64DiesWhenRealtime) {
 }
 #endif // SANITIZER_INTERCEPT_KQUEUE
 
+TEST(TestRtsanInterceptors, MkfifoDiesWhenRealtime) {
+  auto Func = []() { mkfifo("/tmp/rtsan_test_fifo", 0); };
+  ExpectRealtimeDeath(Func, "mkfifo");
+  ExpectNonRealtimeSurvival(Func);
+}
+
+#if __has_builtin(__builtin_available) && SANITIZER_APPLE
+#define MKFIFOAT_AVAILABLE() (__builtin_available(macOS 10.13, *))
+#else
+// We are going to assume this is true until we hit systems where it isn't
+#define MKFIFOAT_AVAILABLE() (true)
+#endif
+
+TEST(TestRtsanInterceptors, MkfifoatDiesWhenRealtime) {
+  if (MKFIFOAT_AVAILABLE()) {
+    auto Func = []() { mkfifoat(0, "/tmp/rtsan_test_fifo", 0); };
+    ExpectRealtimeDeath(Func, "mkfifoat");
+    ExpectNonRealtimeSurvival(Func);
+  }
+}
+
+TEST(TestRtsanInterceptors, PipeDiesWhenRealtime) {
+  int fds[2];
+  auto Func = [&fds]() { pipe(fds); };
+  ExpectRealtimeDeath(Func, "pipe");
+  ExpectNonRealtimeSurvival(Func);
+}
+
 #endif // SANITIZER_POSIX

@cjappl cjappl merged commit fce917d into llvm:main Nov 20, 2024
10 checks passed
@cjappl cjappl deleted the pipe_fifo branch November 20, 2024 16:03
@cjappl
Copy link
Contributor Author

cjappl commented Nov 20, 2024

For anyone discovering this when their darwin builds behaved badly

I am going to revert mkfifoat in just a second, I thought it was introduced in 10.13, but instead it was introduced in 13. I expect this to cause build failures, so I'm reverting it.

That PR is here, being submitted when my local builds pass:
#116997

cjappl added a commit that referenced this pull request Nov 20, 2024
This partially reverts #116915
[fce917d](fce917d)

mkfifoat was improperly guarded against in MacOS systems
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