From 5565fe2f18c9aa0b12c85752bbfe5b54781baa37 Mon Sep 17 00:00:00 2001 From: Chris Apple Date: Mon, 25 Nov 2024 07:23:05 -0800 Subject: [PATCH 1/2] [rtsan] Add ioctl interceptor --- .../lib/rtsan/rtsan_interceptors_posix.cpp | 18 ++++++ .../tests/rtsan_test_interceptors_posix.cpp | 57 ++++++++++++++++++- 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp b/compiler-rt/lib/rtsan/rtsan_interceptors_posix.cpp index 91d023e858ba3..5debf13ab9815 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 *)); + 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 bf6a3a895bd3d..c47a3a12c65b5 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 #include -#include #include #endif @@ -38,12 +37,16 @@ #endif #include +#include +#include #include #include #include #include +#include #include #include +#include #include #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")); From 1253ae18021d1f2caab1634d964c2e129f039c9e Mon Sep 17 00:00:00 2001 From: Chris Apple Date: Tue, 26 Nov 2024 15:15:28 -0800 Subject: [PATCH 2/2] [PR] dtraev - FAIL, remove unneeded return statements --- .../lib/rtsan/tests/rtsan_test_interceptors_posix.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) 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 c47a3a12c65b5..8551424717de6 100644 --- a/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp +++ b/compiler-rt/lib/rtsan/tests/rtsan_test_interceptors_posix.cpp @@ -248,6 +248,7 @@ TEST(TestRtsanInterceptors, SchedYieldDiesWhenRealtime) { /* Filesystem */ + TEST_F(RtsanFileTest, OpenDiesWhenRealtime) { auto Func = [this]() { open(GetTemporaryFilePath(), O_RDONLY); }; ExpectRealtimeDeath(Func, MAYBE_APPEND_64("open")); @@ -396,7 +397,6 @@ TEST(TestRtsanInterceptors, IoctlBehavesWithOutputPointer) { if (sock == -1) { perror("socket"); GTEST_SKIP(); - return; } struct ifaddrs *ifaddr = nullptr; @@ -404,7 +404,6 @@ TEST(TestRtsanInterceptors, IoctlBehavesWithOutputPointer) { perror("getifaddrs"); close(sock); GTEST_SKIP(); - return; } struct ifreq ifr {}; @@ -415,8 +414,7 @@ TEST(TestRtsanInterceptors, IoctlBehavesWithOutputPointer) { perror("ioctl"); close(sock); freeifaddrs(ifaddr); - ASSERT_TRUE(false) << "ioctl failed"; - return; + FAIL(); } freeifaddrs(ifaddr);