Skip to content

Conversation

@cjappl
Copy link
Contributor

@cjappl cjappl commented Nov 25, 2024

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Nov 25, 2024

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

Author: Chris Apple (cjappl)

Changes

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

2 Files Affected:

  • (modified) compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp (+18)
  • (modified) compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp (+55-2)
diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
index 91d023e858ba3b..c3f1badac4f678 100644
--- a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp
@@ -190,6 +190,23 @@ INTERCEPTOR(int, fcntl, int filedes, int cmd, ...) {
   return REAL(fcntl)(filedes, cmd, arg);
 }
 
+INTERCEPTOR(int, ioctl, int filedes, unsigned long request, ...) {
+  __rtsan_notify_intercepted_call("ioctl");
+
+  // See fcntl for discussion on why we use intptr_t
+  // And why we read from va_args on all request types
+  using arg_type = intptr_t;
+  static_assert(sizeof(arg_type) >= sizeof(struct ifreq *)); // used in socket ioctls
+  static_assert(sizeof(arg_type) >= sizeof(int));
+
+  va_list args;
+  va_start(args, request);
+  arg_type arg = va_arg(args, arg_type);
+  va_end(args);
+
+  return REAL(ioctl)(filedes, request, arg);
+}
+
 #if SANITIZER_INTERCEPT_FCNTL64
 INTERCEPTOR(int, fcntl64, int filedes, int cmd, ...) {
   __rtsan_notify_intercepted_call("fcntl64");
@@ -801,6 +818,7 @@ void __rtsan::InitializeInterceptors() {
   RTSAN_MAYBE_INTERCEPT_LSEEK64;
   INTERCEPT_FUNCTION(dup);
   INTERCEPT_FUNCTION(dup2);
+  INTERCEPT_FUNCTION(ioctl);
 
 #if SANITIZER_APPLE
   INTERCEPT_FUNCTION(OSSpinLockLock);
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 bf6a3a895bd3d4..c47a3a12c65b5c 100644
--- a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp
+++ b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp
@@ -20,7 +20,6 @@
 #if SANITIZER_APPLE
 #include <libkern/OSAtomic.h>
 #include <os/lock.h>
-#include <sys/types.h>
 #include <unistd.h>
 #endif
 
@@ -38,12 +37,16 @@
 #endif
 
 #include <fcntl.h>
+#include <ifaddrs.h>
+#include <net/if.h>
 #include <netdb.h>
 #include <poll.h>
 #include <pthread.h>
 #include <stdio.h>
+#include <sys/ioctl.h>
 #include <sys/mman.h>
 #include <sys/socket.h>
+#include <sys/types.h>
 #include <sys/uio.h>
 
 #if _FILE_OFFSET_BITS == 64 && SANITIZER_GLIBC
@@ -245,7 +248,6 @@ TEST(TestRtsanInterceptors, SchedYieldDiesWhenRealtime) {
 /*
     Filesystem
 */
-
 TEST_F(RtsanFileTest, OpenDiesWhenRealtime) {
   auto Func = [this]() { open(GetTemporaryFilePath(), O_RDONLY); };
   ExpectRealtimeDeath(Func, MAYBE_APPEND_64("open"));
@@ -373,6 +375,57 @@ class RtsanOpenedFileTest : public RtsanFileTest {
   int fd = -1;
 };
 
+TEST(TestRtsanInterceptors, IoctlDiesWhenRealtime) {
+  auto Func = []() { ioctl(0, FIONREAD); };
+  ExpectRealtimeDeath(Func, "ioctl");
+  ExpectNonRealtimeSurvival(Func);
+}
+
+TEST_F(RtsanOpenedFileTest, IoctlBehavesWithOutputArg) {
+  int arg{};
+  ioctl(GetOpenFd(), FIONREAD, &arg);
+
+  EXPECT_THAT(arg, Ge(0));
+}
+
+TEST(TestRtsanInterceptors, IoctlBehavesWithOutputPointer) {
+  // These initial checks just see if we CAN run these tests.
+  // If we can't (can't open a socket, or can't find an interface, just
+  // gracefully skip.
+  int sock = socket(AF_INET, SOCK_STREAM, 0);
+  if (sock == -1) {
+    perror("socket");
+    GTEST_SKIP();
+    return;
+  }
+
+  struct ifaddrs *ifaddr = nullptr;
+  if (getifaddrs(&ifaddr) == -1 || ifaddr == nullptr) {
+    perror("getifaddrs");
+    close(sock);
+    GTEST_SKIP();
+    return;
+  }
+
+  struct ifreq ifr {};
+  strncpy(ifr.ifr_name, ifaddr->ifa_name, IFNAMSIZ - 1);
+
+  int retval = ioctl(sock, SIOCGIFADDR, &ifr);
+  if (retval == -1) {
+    perror("ioctl");
+    close(sock);
+    freeifaddrs(ifaddr);
+    ASSERT_TRUE(false) << "ioctl failed";
+    return;
+  }
+
+  freeifaddrs(ifaddr);
+  close(sock);
+
+  ASSERT_THAT(ifr.ifr_addr.sa_data, NotNull());
+  ASSERT_THAT(ifr.ifr_addr.sa_family, Eq(AF_INET));
+}
+
 TEST_F(RtsanOpenedFileTest, LseekDiesWhenRealtime) {
   auto Func = [this]() { lseek(GetOpenFd(), 0, SEEK_SET); };
   ExpectRealtimeDeath(Func, MAYBE_APPEND_64("lseek"));

EXPECT_THAT(arg, Ge(0));
}

TEST(TestRtsanInterceptors, IoctlBehavesWithOutputPointer) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test (and the one above) exists to ensure we pass on the arg correctly. It's just a simple "lights on" test of ioctl getting info from a socket

@github-actions
Copy link

github-actions bot commented Nov 25, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@cjappl cjappl merged commit 3a8b28f into llvm:main Nov 26, 2024
4 of 6 checks passed
@cjappl cjappl deleted the ioctl branch November 26, 2024 23:17
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