compiler-rt: intercept fortified read/pread wrappers

[maflcko]

Add TSAN/common interceptors for glibc fortified read/pread calls, so that they follow the same blocking and signal-handling path as the plain libc symbols.

The regression test from #77789 for read, when compiled with -D_FORTIFY_SOURCE=3 fails without the new interceptors.

I have a regression test for pread(64) as well, but I am not sure if there is much value in adding it.

I've added a read smoke test in sanitizer common.

[llvmorg-github-actions]

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

Author: maflcko

Changes

Add TSAN/common interceptors for glibc fortified read/pread calls, so that they follow the same blocking and signal-handling path as the plain libc symbols.

The regression test from #77789 for read, when compiled with -D_FORTIFY_SOURCE=3 fails without the new interceptors.

I have a regression test for pread(64) as well, but I am not sure if there is much value in adding it.

---

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

3 Files Affected:

• (modified) compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc (+55)
• (modified) compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h (+3)
• (modified) compiler-rt/test/tsan/signal_in_read.c (+1)

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
index c76010e77d1fa..412d5f9b37923 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
@@ -1026,6 +1026,23 @@ INTERCEPTOR(SSIZE_T, read, int fd, void *ptr, SIZE_T count) {
 #define INIT_READ
 #endif
 
+#if SANITIZER_INTERCEPT___READ_CHK
+INTERCEPTOR(SSIZE_T, __read_chk, int fd, void *ptr, SIZE_T count,
+            SIZE_T buflen) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, __read_chk, fd, ptr, count, buflen);
+  COMMON_INTERCEPTOR_FD_ACCESS(ctx, fd);
+  SSIZE_T res = COMMON_INTERCEPTOR_BLOCK_REAL(__read_chk)(fd, ptr, count,
+                                                          buflen);
+  if (res > 0) COMMON_INTERCEPTOR_WRITE_RANGE(ctx, ptr, res);
+  if (res >= 0 && fd >= 0) COMMON_INTERCEPTOR_FD_ACQUIRE(ctx, fd);
+  return res;
+}
+#define INIT___READ_CHK COMMON_INTERCEPT_FUNCTION(__read_chk)
+#else
+#define INIT___READ_CHK
+#endif
+
 #if SANITIZER_INTERCEPT_FREAD
 INTERCEPTOR(SIZE_T, fread, void *ptr, SIZE_T size, SIZE_T nmemb, void *file) {
   // libc file streams can call user-supplied functions, see fopencookie.
@@ -1061,6 +1078,23 @@ INTERCEPTOR(SSIZE_T, pread, int fd, void *ptr, SIZE_T count, OFF_T offset) {
 #define INIT_PREAD
 #endif
 
+#if SANITIZER_INTERCEPT___PREAD_CHK
+INTERCEPTOR(SSIZE_T, __pread_chk, int fd, void *ptr, SIZE_T count,
+            OFF_T offset, SIZE_T buflen) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, __pread_chk, fd, ptr, count, offset, buflen);
+  COMMON_INTERCEPTOR_FD_ACCESS(ctx, fd);
+  SSIZE_T res = COMMON_INTERCEPTOR_BLOCK_REAL(__pread_chk)(fd, ptr, count,
+                                                           offset, buflen);
+  if (res > 0) COMMON_INTERCEPTOR_WRITE_RANGE(ctx, ptr, res);
+  if (res >= 0 && fd >= 0) COMMON_INTERCEPTOR_FD_ACQUIRE(ctx, fd);
+  return res;
+}
+#define INIT___PREAD_CHK COMMON_INTERCEPT_FUNCTION(__pread_chk)
+#else
+#define INIT___PREAD_CHK
+#endif
+
 #if SANITIZER_INTERCEPT_PREAD64
 INTERCEPTOR(SSIZE_T, pread64, int fd, void *ptr, SIZE_T count, OFF64_T offset) {
   void *ctx;
@@ -1079,6 +1113,24 @@ INTERCEPTOR(SSIZE_T, pread64, int fd, void *ptr, SIZE_T count, OFF64_T offset) {
 #define INIT_PREAD64
 #endif
 
+#if SANITIZER_INTERCEPT___PREAD64_CHK
+INTERCEPTOR(SSIZE_T, __pread64_chk, int fd, void *ptr, SIZE_T count,
+            OFF64_T offset, SIZE_T buflen) {
+  void *ctx;
+  COMMON_INTERCEPTOR_ENTER(ctx, __pread64_chk, fd, ptr, count, offset,
+                           buflen);
+  COMMON_INTERCEPTOR_FD_ACCESS(ctx, fd);
+  SSIZE_T res = COMMON_INTERCEPTOR_BLOCK_REAL(__pread64_chk)(fd, ptr, count,
+                                                             offset, buflen);
+  if (res > 0) COMMON_INTERCEPTOR_WRITE_RANGE(ctx, ptr, res);
+  if (res >= 0 && fd >= 0) COMMON_INTERCEPTOR_FD_ACQUIRE(ctx, fd);
+  return res;
+}
+#define INIT___PREAD64_CHK COMMON_INTERCEPT_FUNCTION(__pread64_chk)
+#else
+#define INIT___PREAD64_CHK
+#endif
+
 #if SANITIZER_INTERCEPT_READV
 INTERCEPTOR_WITH_SUFFIX(SSIZE_T, readv, int fd, __sanitizer_iovec *iov,
                         int iovcnt) {
@@ -10431,9 +10483,12 @@ static void InitializeCommonInterceptors() {
   INIT_MEMRCHR;
   INIT_MEMMEM;
   INIT_READ;
+  INIT___READ_CHK;
   INIT_FREAD;
   INIT_PREAD;
+  INIT___PREAD_CHK;
   INIT_PREAD64;
+  INIT___PREAD64_CHK;
   INIT_READV;
   INIT_PREADV;
   INIT_PREADV64;
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h b/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
index 9f6fc4e9b9bfa..702c960434a0f 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_platform_interceptors.h
@@ -201,6 +201,9 @@ SANITIZER_WEAK_IMPORT void *aligned_alloc(__sanitizer::usize __alignment,
 
 #define SANITIZER_INTERCEPT_READ SI_POSIX
 #define SANITIZER_INTERCEPT_PREAD SI_POSIX
+#define SANITIZER_INTERCEPT___READ_CHK SI_GLIBC
+#define SANITIZER_INTERCEPT___PREAD_CHK SI_GLIBC
+#define SANITIZER_INTERCEPT___PREAD64_CHK SI_GLIBC
 #define SANITIZER_INTERCEPT_WRITE SI_POSIX
 #define SANITIZER_INTERCEPT_PWRITE SI_POSIX
 
diff --git a/compiler-rt/test/tsan/signal_in_read.c b/compiler-rt/test/tsan/signal_in_read.c
index ec50d9d021745..82f9a58e46da5 100644
--- a/compiler-rt/test/tsan/signal_in_read.c
+++ b/compiler-rt/test/tsan/signal_in_read.c
@@ -1,4 +1,5 @@
 // RUN: %clang_tsan -O1 %s -o %t && %run %t 2>&1 | FileCheck %s
+// RUN: %clang_tsan -O1 -D_FORTIFY_SOURCE=3 %s -o %t.fortify && %run %t.fortify 2>&1 | FileCheck %s
 
 #include "test.h"
 

[github-actions]

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

[github-actions]

🐧 Linux x64 Test Results

• 6592 tests passed
• 1315 tests skipped

✅ The build succeeded and all tests passed.

[vitalybuka]

Could you please add a basic test into sanitizer_common

[maflcko]

Sure, added a smoke test to cover the new code. (The pre-existing tsan test remains as regression test)

[maflcko]

Going further, those aren't an issue that I ran into, but I wonder if it makes sense to wrap some others as well?

$ cat /tmp/repro.sh 
#!/usr/bin/env bash

cat /tmp/repro.cpp

clang++ -O1 -g -fsanitize=thread -D_FORTIFY_SOURCE=0 /tmp/repro.cpp -pthread -o /tmp/repro_fort0
clang++ -O1 -g -fsanitize=thread -D_FORTIFY_SOURCE=3 /tmp/repro.cpp -pthread -o /tmp/repro_fort3

for case in confstr getgroups ttyname_r getlogin_r gethostname getdomainname getcwd; do
  echo "===="
  echo "CASE=$case"
  REPRO_CASE=$case timeout --verbose --signal KILL 2s /tmp/repro_fort0
  echo " fort0=$?"
  REPRO_CASE=$case timeout --verbose --signal KILL 2s /tmp/repro_fort3
  echo " fort3=$?"
done





$ /tmp/repro.sh 
#include <csignal>
#include <cstdlib>
#include <cstring>
#include <unistd.h>
#include <grp.h>

volatile sig_atomic_t got_sig = 0;
void handler(int) { got_sig = 1; }

template <typename Fn>
static int run_repro(Fn fn)
{
    signal(SIGALRM, handler);
    ualarm(100000, 0);
    while (!got_sig) {
        (void)fn();
    }
    return 0;
}

int main()
{
    const char *kind = getenv("REPRO_CASE");
    if (!kind) kind = "confstr";

    if (strcmp(kind, "confstr") == 0) {
        alignas(16) char buf[1 << 20];
        return run_repro([&]() { return confstr(_CS_PATH, buf, sizeof(buf)); });
    }
    if (strcmp(kind, "getgroups") == 0) {
        alignas(16) gid_t buf[1024];
        return run_repro([&]() { return getgroups(1024, buf); });
    }
    if (strcmp(kind, "ttyname_r") == 0) {
        alignas(16) char buf[1 << 20];
        return run_repro([&]() { return ttyname_r(STDIN_FILENO, buf, sizeof(buf)); });
    }
    if (strcmp(kind, "getlogin_r") == 0) {
        alignas(16) char buf[1 << 20];
        return run_repro([&]() { return getlogin_r(buf, sizeof(buf)); });
    }
    if (strcmp(kind, "gethostname") == 0) {
        alignas(16) char buf[1 << 20];
        return run_repro([&]() { return gethostname(buf, sizeof(buf)); });
    }
    if (strcmp(kind, "getdomainname") == 0) {
        alignas(16) char buf[1 << 20];
        return run_repro([&]() { return getdomainname(buf, sizeof(buf)); });
    }
    if (strcmp(kind, "getcwd") == 0) {
        alignas(16) char buf[1 << 20];
        return run_repro([&]() { return getcwd(buf, sizeof(buf)) ? 0 : -1; });
    }
    return 1;
}
====
CASE=confstr
 fort0=0
timeout: sending signal KILL to command '/tmp/repro_fort3'
 fort3=124
====
CASE=getgroups
 fort0=0
timeout: sending signal KILL to command '/tmp/repro_fort3'
 fort3=124
====
CASE=ttyname_r
 fort0=0
timeout: sending signal KILL to command '/tmp/repro_fort3'
 fort3=124
====
CASE=getlogin_r
 fort0=0
 fort3=0
====
CASE=gethostname
timeout: sending signal KILL to command '/tmp/repro_fort0'
 fort0=124
timeout: sending signal KILL to command '/tmp/repro_fort3'
 fort3=124
====
CASE=getdomainname
timeout: sending signal KILL to command '/tmp/repro_fort0'
 fort0=124
timeout: sending signal KILL to command '/tmp/repro_fort3'
 fort3=124
====
CASE=getcwd
 fort0=0
timeout: sending signal KILL to command '/tmp/repro_fort3'
 fort3=124