From c26a43ea30c65a39d184decbc90367f8f857339f Mon Sep 17 00:00:00 2001 From: Chris Apple <14171107+cjappl@users.noreply.github.com> Date: Fri, 31 May 2024 13:20:26 -0700 Subject: [PATCH 1/2] Fix issue where fcntl was not using last arg correctly --- .../lib/radsan/radsan_interceptors.cpp | 10 +++++-- .../radsan/tests/radsan_test_interceptors.cpp | 26 +++++++++++++++++-- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/compiler-rt/lib/radsan/radsan_interceptors.cpp b/compiler-rt/lib/radsan/radsan_interceptors.cpp index 8acdd8504607b..1c25c8ff8c00b 100644 --- a/compiler-rt/lib/radsan/radsan_interceptors.cpp +++ b/compiler-rt/lib/radsan/radsan_interceptors.cpp @@ -88,10 +88,16 @@ INTERCEPTOR(int, fcntl, int filedes, int cmd, ...) { va_list args; va_start(args, cmd); - void *arg = va_arg(args, void *); + + // bit of a hack here, we need to throw the argument into a variable that will + // hold the largest of the possible argument types. It is then assumed that + // fcntl will cast it properly. + const unsigned long arg = va_arg(args, unsigned long); + int result = REAL(fcntl)(filedes, cmd, arg); + va_end(args); - return fcntl(filedes, cmd, arg); + return result; } INTERCEPTOR(int, close, int filedes) { diff --git a/compiler-rt/lib/radsan/tests/radsan_test_interceptors.cpp b/compiler-rt/lib/radsan/tests/radsan_test_interceptors.cpp index 3dabd38fec72a..b62dd7af57548 100644 --- a/compiler-rt/lib/radsan/tests/radsan_test_interceptors.cpp +++ b/compiler-rt/lib/radsan/tests/radsan_test_interceptors.cpp @@ -213,11 +213,11 @@ TEST(TestRadsanInterceptors, fcntlFlockDiesWhenRealtime) { ASSERT_THAT(fd, Ne(-1)); auto func = [fd]() { - struct flock lock{}; + struct flock lock {}; lock.l_type = F_RDLCK; lock.l_whence = SEEK_SET; lock.l_start = 0; - lock.l_len = 0; + lock.l_len = 0; lock.l_pid = ::getpid(); ASSERT_THAT(fcntl(fd, F_GETLK, &lock), Eq(0)); @@ -230,6 +230,28 @@ TEST(TestRadsanInterceptors, fcntlFlockDiesWhenRealtime) { std::remove(temporary_file_path()); } +TEST(TestRadsanInterceptors, fcntlSetFdDiesWhenRealtime) { + int fd = creat(temporary_file_path(), S_IRUSR | S_IWUSR); + ASSERT_THAT(fd, Ne(-1)); + + auto func = [fd]() { + int old_flags = fcntl(fd, F_GETFD); + ASSERT_THAT(fcntl(fd, F_SETFD, FD_CLOEXEC), Eq(0)); + + int flags = fcntl(fd, F_GETFD); + ASSERT_THAT(flags, Ne(-1)); + ASSERT_THAT(flags & FD_CLOEXEC, Eq(FD_CLOEXEC)); + + ASSERT_THAT(fcntl(fd, F_SETFD, old_flags), Eq(0)); + ASSERT_THAT(fcntl(fd, F_GETFD), Eq(old_flags)); + }; + + expectRealtimeDeath(func, "fcntl"); + expectNonrealtimeSurvival(func); + + close(fd); +} + TEST(TestRadsanInterceptors, closeDiesWhenRealtime) { auto func = []() { close(0); }; expectRealtimeDeath(func, "close"); From 47e935032f791b2956b6cfb6873cd33af5a1854e Mon Sep 17 00:00:00 2001 From: Chris Apple <14171107+cjappl@users.noreply.github.com> Date: Fri, 31 May 2024 14:11:53 -0700 Subject: [PATCH 2/2] Comment update --- compiler-rt/lib/radsan/radsan_interceptors.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/compiler-rt/lib/radsan/radsan_interceptors.cpp b/compiler-rt/lib/radsan/radsan_interceptors.cpp index 1c25c8ff8c00b..2ff174d5118fc 100644 --- a/compiler-rt/lib/radsan/radsan_interceptors.cpp +++ b/compiler-rt/lib/radsan/radsan_interceptors.cpp @@ -89,9 +89,13 @@ INTERCEPTOR(int, fcntl, int filedes, int cmd, ...) { va_list args; va_start(args, cmd); - // bit of a hack here, we need to throw the argument into a variable that will - // hold the largest of the possible argument types. It is then assumed that - // fcntl will cast it properly. + // Following precedent here. The linux source (fcntl.c, do_fcntl) accepts the + // final argument in a variable that will hold the largest of the possible + // argument types (pointers and ints are typical in fcntl) It is then assumed + // that the implementation of fcntl will cast it properly depending on cmd. + // + // This is also similar to what is done in + // sanitizer_common/sanitizer_common_syscalls.inc const unsigned long arg = va_arg(args, unsigned long); int result = REAL(fcntl)(filedes, cmd, arg);