[sanitizers] Fix interception of GLibc regexec

[arichardson]

Summary

Previously, on GLibc systems, the interceptor was calling compat_regexec
(regexec@GLIBC_2.2.5) insead of the newer regexec (regexec@GLIBC_2.3.4).
The __compat_regexec strips the REG_STARTEND flag but does not report an
error if other flags are present. This can result in infinite loops for
programs that use REG_STARTEND to find all matches inside a buffer (since
ignoring REG_STARTEND means that the search always starts from the first
character).

The underlying issue is that GLibc's dlsym(RTLD_NEXT, ...) appears to
always return the oldest versioned symbol instead of the default. This
means it does not match the behaviour of dlsym(RTLD_DEFAULT, ...) or the
behaviour documented in the manpage.

It appears a similar issue was encountered with realpath and worked around
in rG77ef78a0a5dbaa364529bd05ed7a7bd9a71dd8d4.

See also https://sourceware.org/bugzilla/show_bug.cgi?id=14932 and
https://sourceware.org/bugzilla/show_bug.cgi?id=1319.

Fixes https://github.com/google/sanitizers/issues/1371

[vitalybuka]

compiler-rt/test/sanitizer_common/TestCases/Posix/regex_startend.cpp

2

Test can be simplified by removing FileCheck and just using asserts

[arichardson]

• simplify test using abort()

Note: incremental ninja check-sanitizer builds will fail this test since the change to the .inc does not trigger a rebuild of the runtime libraries. CMake bug?

[vitalybuka]

In D96348#2568009, @arichardson wrote:

• simplify test using abort()

Note: incremental ninja check-sanitizer builds will fail this test since the change to the .inc does not trigger a rebuild of the runtime libraries. CMake bug?

it's possible

compiler-rt/test/sanitizer_common/TestCases/Posix/regex_startend.cpp

27–40

I assumed you can remove printfs and most of this ifs and use asserts

[arichardson]

compiler-rt/test/sanitizer_common/TestCases/Posix/regex_startend.cpp

27–40

Yeah I can definitely do this, but I find the prints quite useful when things do go wrong. Happy to change it if you prefer,

[vitalybuka]

compiler-rt/test/sanitizer_common/TestCases/Posix/regex_startend.cpp

27–40

assert(expected != nullptr);
assert((size_t)matchlen == strlen(expected));
assert(strncmp(matchstart, expected, matchlen) == 0);

[marxin]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

7801

I think this is problematic because there are targets that don't define regexec@GLIBC_2.3.4.
Please see my target selection in https://reviews.llvm.org/D95864.

[arichardson]

• Update to do an unversioned fallback (needed for non-x86_64, see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98920)

[arichardson]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

7801

I've added logic to do an unversioned fallback. To me this seems a bit more maintainable than encoding the target list (since that needs to be updated for every new architecture).

[ro]

The new test FAILs on Solaris:

SanitizerCommon-asan-i386-SunOS :: Posix/regex_startend.cpp
SanitizerCommon-ubsan-i386-SunOS :: Posix/regex_startend.cpp
SanitizerCommon-ubsan-x86_64-SunOS :: Posix/regex_startend.cpp

with

/vol/llvm/src/llvm-project/dist/compiler-rt/test/sanitizer_common/TestCases/Posix/regex_startend.cpp:22:44: error: use of undeclared identifier 'REG_STARTEND'
  int rv = regexec(preg, string, 1, match, REG_STARTEND);

This is no wonder given Linux regex(3) states

REG_STARTEND
       Use   pmatch[0]   on   the   input   string,  starting  at  byte
       pmatch[0].rm_so and ending before  byte  pmatch[0].rm_eo.   This
       allows  matching  embedded  NUL  bytes and avoids a strlen(3) on
       large strings.  It does not use nmatch on input,  and  does  not
       change REG_NOTBOL or REG_NEWLINE processing.  This flag is a BSD
       extension, not present in POSIX.

If the test requires glibc targets, it should be restricted to such.

[arichardson]

In D96348#2613433, @ro wrote:

The new test FAILs on Solaris:

SanitizerCommon-asan-i386-SunOS :: Posix/regex_startend.cpp
SanitizerCommon-ubsan-i386-SunOS :: Posix/regex_startend.cpp
SanitizerCommon-ubsan-x86_64-SunOS :: Posix/regex_startend.cpp

with

/vol/llvm/src/llvm-project/dist/compiler-rt/test/sanitizer_common/TestCases/Posix/regex_startend.cpp:22:44: error: use of undeclared identifier 'REG_STARTEND'
  int rv = regexec(preg, string, 1, match, REG_STARTEND);

This is no wonder given Linux regex(3) states

REG_STARTEND
       Use   pmatch[0]   on   the   input   string,  starting  at  byte
       pmatch[0].rm_so and ending before  byte  pmatch[0].rm_eo.   This
       allows  matching  embedded  NUL  bytes and avoids a strlen(3) on
       large strings.  It does not use nmatch on input,  and  does  not
       change REG_NOTBOL or REG_NEWLINE processing.  This flag is a BSD
       extension, not present in POSIX.

If the test requires glibc targets, it should be restricted to such.

It is a common extension that is present on all BSDs, macOS, Linux and possibly other operating systems. It think the best solution would be to add UNSUPPORTED: solaris or wrap the entire test in #ifdef REG_STARTEND and add a dummy main method?

[ro]

In D96348#2613440, @arichardson wrote:

It is a common extension that is present on all BSDs, macOS, Linux and possibly other operating systems. It think the best solution would be to add UNSUPPORTED: solaris or wrap the entire test in #ifdef REG_STARTEND and add a dummy main method?

It may be common, but it's nonstandard and certainly not univeral: I've just checked AIX 7.2 which doesn't have it either. I believe wrapping in #ifdef REG_STARTEND is better than repeatedly adjusting UNSUPPORTED clauses, provided the test can work on every target that has REG_STARTEND.

[arichardson]

In D96348#2613450, @ro wrote:

In D96348#2613440, @arichardson wrote:

It is a common extension that is present on all BSDs, macOS, Linux and possibly other operating systems. It think the best solution would be to add UNSUPPORTED: solaris or wrap the entire test in #ifdef REG_STARTEND and add a dummy main method?

It may be common, but it's nonstandard and certainly not univeral: I've just checked AIX 7.2 which doesn't have it either. I believe wrapping in #ifdef REG_STARTEND is better than repeatedly adjusting UNSUPPORTED clauses, provided the test can work on every target that has REG_STARTEND.

The test will work on all targets where the host libc supports REG_STARTEND (unless of course there is a bug related to sanitizer interception as was the case on x86_64 glibc).

[ro]

In D96348#2613450, @ro wrote:

In D96348#2613440, @arichardson wrote:

It is a common extension that is present on all BSDs, macOS, Linux and possibly other operating systems. It think the best solution would be to add UNSUPPORTED: solaris or wrap the entire test in #ifdef REG_STARTEND and add a dummy main method?

It may be common, but it's nonstandard and certainly not univeral: I've just checked AIX 7.2 which doesn't have it either. I believe wrapping in #ifdef REG_STARTEND is better than repeatedly adjusting UNSUPPORTED clauses, provided the test can work on every target that has REG_STARTEND.

Done now as D98425.

[marxin]

I would like to point to an issue reported on s390x:
https://github.com/google/sanitizers/issues/1390

[MaskRay]

The underlying issue is that GLibc's dlsym(RTLD_NEXT, ...) appears to always return the oldest versioned symbol instead of the default. This means it does not match the behaviour of dlsym(RTLD_DEFAULT, ...) or the behaviour documented in the manpage.

I fixed https://sourceware.org/bugzilla/show_bug.cgi?id=14932 today, so we shall not need versioned interceptors in the future (glibc >= 2.36).
The current versioned interceptors are still needed for glibc<2.36 compatibility.

compiler-rt/lib/asan/asan_interceptors.h

if (!INTERCEPT_FUNCTION(name))                                       \
       VReport(1, "AddressSanitizer: failed to intercept '%s'\n", #name); \
   } while (0)
 #define ASAN_INTERCEPT_FUNC_VER(name, ver)                                  \
   do {                                                                      \
     if (!INTERCEPT_FUNCTION_VER(name, ver))                                 \
       VReport(1, "AddressSanitizer: failed to intercept '%s@@%s'\n", #name, \
               #ver);                                                        \
   } while (0)
+#define ASAN_INTERCEPT_FUNC_VER_UNVERSIONED_FALLBACK(name, ver)              \
+  do {                                                                       \
+    if (!INTERCEPT_FUNCTION_VER(name, ver) && !INTERCEPT_FUNCTION(name))     \
+      VReport(1, "AddressSanitizer: failed to intercept '%s@@%s' or '%s'\n", \
+              #name, #ver, #name);                                           \
+  } while (0)
+
 #else
 // OS X interceptors don't need to be initialized with INTERCEPT_FUNCTION.
 #define ASAN_INTERCEPT_FUNC(name)
 #endif  // SANITIZER_MAC
 
 #endif  // !SANITIZER_FUCHSIA
 
 #endif  // ASAN_INTERCEPTORS_H

compiler-rt/lib/asan/asan_interceptors.cpp

 DECLARE_REAL_AND_INTERCEPTOR(void, free, void *)
 
 #define ASAN_INTERCEPTOR_ENTER(ctx, func)                                      \
   AsanInterceptorContext _ctx = {#func};                                       \
   ctx = (void *)&_ctx;                                                         \
   (void) ctx;                                                                  \
 
 #define COMMON_INTERCEPT_FUNCTION(name) ASAN_INTERCEPT_FUNC(name)
-#define COMMON_INTERCEPT_FUNCTION_VER(name, ver)                          \
+#define COMMON_INTERCEPT_FUNCTION_VER(name, ver) \
   ASAN_INTERCEPT_FUNC_VER(name, ver)
+#define COMMON_INTERCEPT_FUNCTION_VER_UNVERSIONED_FALLBACK(name, ver) \
+  ASAN_INTERCEPT_FUNC_VER_UNVERSIONED_FALLBACK(name, ver)
 #define COMMON_INTERCEPTOR_WRITE_RANGE(ctx, ptr, size) \
   ASAN_WRITE_RANGE(ctx, ptr, size)
 #define COMMON_INTERCEPTOR_READ_RANGE(ctx, ptr, size) \
   ASAN_READ_RANGE(ctx, ptr, size)
 #define COMMON_INTERCEPTOR_ENTER(ctx, func, ...)                               \
   ASAN_INTERCEPTOR_ENTER(ctx, func);                                           \
   do {                                                                         \
     if (asan_init_is_running)                                                  \

 #endif
   // Indirectly intercept std::rethrow_exception.
 #if ASAN_INTERCEPT__UNWIND_SJLJ_RAISEEXCEPTION
   INTERCEPT_FUNCTION(_Unwind_SjLj_RaiseException);
 #endif
 
   // Intercept threading-related functions
 #if ASAN_INTERCEPT_PTHREAD_CREATE
+// TODO: this should probably have an unversioned fallback for newer arches?
 #if defined(ASAN_PTHREAD_CREATE_VERSION)
   ASAN_INTERCEPT_FUNC_VER(pthread_create, ASAN_PTHREAD_CREATE_VERSION);
 #else
   ASAN_INTERCEPT_FUNC(pthread_create);
 #endif
   ASAN_INTERCEPT_FUNC(pthread_join);
 #endif
 

compiler-rt/lib/memprof/memprof_interceptors.h

do {                                                                         \
     if (!INTERCEPT_FUNCTION(name))                                             \
       VReport(1, "MemProfiler: failed to intercept '%s'\n'", #name);           \
   } while (0)
 #define MEMPROF_INTERCEPT_FUNC_VER(name, ver)                                  \
   do {                                                                         \
     if (!INTERCEPT_FUNCTION_VER(name, ver))                                    \
       VReport(1, "MemProfiler: failed to intercept '%s@@%s'\n", #name, #ver);  \
   } while (0)
+#define MEMPROF_INTERCEPT_FUNC_VER_UNVERSIONED_FALLBACK(name, ver)             \
+  do {                                                                         \
+    if (!INTERCEPT_FUNCTION_VER(name, ver) && !INTERCEPT_FUNCTION(name))       \
+      VReport(1, "MemProfiler: failed to intercept '%s@@%s' or '%s'\n", #name, \
+              #ver, #name);                                                    \
+  } while (0)
 
 #endif // MEMPROF_INTERCEPTORS_H

compiler-rt/lib/memprof/memprof_interceptors.cpp

 
 #define MEMPROF_INTERCEPTOR_ENTER(ctx, func)                                   \
   ctx = 0;                                                                     \
   (void)ctx;
 
 #define COMMON_INTERCEPT_FUNCTION(name) MEMPROF_INTERCEPT_FUNC(name)
 #define COMMON_INTERCEPT_FUNCTION_VER(name, ver)                               \
   MEMPROF_INTERCEPT_FUNC_VER(name, ver)
+#define COMMON_INTERCEPT_FUNCTION_VER_UNVERSIONED_FALLBACK(name, ver)          \
+  MEMPROF_INTERCEPT_FUNC_VER_UNVERSIONED_FALLBACK(name, ver)
 #define COMMON_INTERCEPTOR_WRITE_RANGE(ctx, ptr, size)                         \
   MEMPROF_WRITE_RANGE(ptr, size)
 #define COMMON_INTERCEPTOR_READ_RANGE(ctx, ptr, size)                          \
   MEMPROF_READ_RANGE(ptr, size)
 #define COMMON_INTERCEPTOR_ENTER(ctx, func, ...)                               \
   MEMPROF_INTERCEPTOR_ENTER(ctx, func);                                        \
   do {                                                                         \
     if (memprof_init_is_running)                                               \

compiler-rt/lib/msan/msan_interceptors.cpp

#define MSAN_INTERCEPT_FUNC(name)                                       \
   } while (0)
 
 #define MSAN_INTERCEPT_FUNC_VER(name, ver)                                 \
   do {                                                                     \
     if (!INTERCEPT_FUNCTION_VER(name, ver))                                \
       VReport(1, "MemorySanitizer: failed to intercept '%s@@%s'\n", #name, \
               #ver);                                                       \
   } while (0)
+#define MSAN_INTERCEPT_FUNC_VER_UNVERSIONED_FALLBACK(name, ver)             \
+  do {                                                                      \
+    if (!INTERCEPT_FUNCTION_VER(name, ver) && !INTERCEPT_FUNCTION(name))    \
+      VReport(1, "MemorySanitizer: failed to intercept '%s@@%s' or '%s'\n", \
+              #name, #ver, #name);                                          \
+  } while (0)
 
 #define COMMON_INTERCEPT_FUNCTION(name) MSAN_INTERCEPT_FUNC(name)
-#define COMMON_INTERCEPT_FUNCTION_VER(name, ver)                          \
+#define COMMON_INTERCEPT_FUNCTION_VER(name, ver) \
   MSAN_INTERCEPT_FUNC_VER(name, ver)
+#define COMMON_INTERCEPT_FUNCTION_VER_UNVERSIONED_FALLBACK(name, ver) \
+  MSAN_INTERCEPT_FUNC_VER_UNVERSIONED_FALLBACK(name, ver)
 #define COMMON_INTERCEPTOR_UNPOISON_PARAM(count)  \
   UnpoisonParam(count)
 #define COMMON_INTERCEPTOR_WRITE_RANGE(ctx, ptr, size) \
   __msan_unpoison(ptr, size)
 #define COMMON_INTERCEPTOR_READ_RANGE(ctx, ptr, size) \
   CHECK_UNPOISONED_CTX(ctx, ptr, size)
 #define COMMON_INTERCEPTOR_INITIALIZE_RANGE(ptr, size) \
   __msan_unpoison(ptr, size)

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

 #ifdef SANITIZER_NLDBL_VERSION
 #define COMMON_INTERCEPT_FUNCTION_LDBL(fn)                          \
     COMMON_INTERCEPT_FUNCTION_VER(fn, SANITIZER_NLDBL_VERSION)
 #else
 #define COMMON_INTERCEPT_FUNCTION_LDBL(fn)                          \
     COMMON_INTERCEPT_FUNCTION(fn)
 #endif
 
+#if SANITIZER_GLIBC
+// If we could not find the versioned symbol, fall back to an unversioned
+// lookup. This is needed to work around a GLibc bug that causes dlsym
+// with RTLD_NEXT to return the oldest versioned symbol.
+// See https://sourceware.org/bugzilla/show_bug.cgi?id=14932.
+// For certain symbols (e.g. regexec) we have to perform a versioned lookup,
+// but that versioned symbol will only exist for architectures where the
+// oldest Glibc version pre-dates support for that architecture.
+// For example, regexec@GLIBC_2.3.4 exists on x86_64, but not RISC-V.
+// See also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98920.
+#define COMMON_INTERCEPT_FUNCTION_GLIBC_VER_MIN(fn, ver) \
+  COMMON_INTERCEPT_FUNCTION_VER_UNVERSIONED_FALLBACK(fn, ver)
+#else
+#define COMMON_INTERCEPT_FUNCTION_GLIBC_VER_MIN(fn, ver) \
+  COMMON_INTERCEPT_FUNCTION(fn)
+#endif
+
 #ifndef COMMON_INTERCEPTOR_MEMSET_IMPL
 #define COMMON_INTERCEPTOR_MEMSET_IMPL(ctx, dst, v, size) \
   {                                                       \
     if (COMMON_INTERCEPTOR_NOTHING_IS_INITIALIZED)        \
       return internal_memset(dst, v, size);               \
     COMMON_INTERCEPTOR_ENTER(ctx, memset, dst, v, size);  \
     if (common_flags()->intercept_intrin)                 \
       COMMON_INTERCEPTOR_WRITE_RANGE(ctx, dst, size);     \
INTERCEPTOR(void, regfree, const void *preg) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, regfree, preg);
   if (preg)
     COMMON_INTERCEPTOR_READ_RANGE(ctx, preg, struct_regex_sz);
   REAL(regfree)(preg);
 }
 #define INIT_REGEX                                                             \
   COMMON_INTERCEPT_FUNCTION(regcomp);                                          \
-  COMMON_INTERCEPT_FUNCTION(regexec);                                          \
+  COMMON_INTERCEPT_FUNCTION_GLIBC_VER_MIN(regexec, "GLIBC_2.3.4");             \
   COMMON_INTERCEPT_FUNCTION(regerror);                                         \
   COMMON_INTERCEPT_FUNCTION(regfree);
 #else
 #define INIT_REGEX
 #endif
 
 #if SANITIZER_INTERCEPT_REGEXSUB
 INTERCEPTOR(SSIZE_T, regnsub, char *buf, SIZE_T bufsiz, const char *sub,

compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp

 #define NEED_TLS_GET_ADDR
 #endif
 #undef SANITIZER_INTERCEPT_TLS_GET_ADDR
 #undef SANITIZER_INTERCEPT_PTHREAD_SIGMASK
 
 #define COMMON_INTERCEPT_FUNCTION(name) INTERCEPT_FUNCTION(name)
 #define COMMON_INTERCEPT_FUNCTION_VER(name, ver)                          \
   INTERCEPT_FUNCTION_VER(name, ver)
+#define COMMON_INTERCEPT_FUNCTION_VER_UNVERSIONED_FALLBACK(name, ver) \
+  (INTERCEPT_FUNCTION_VER(name, ver) || INTERCEPT_FUNCTION(name))
 
 #define COMMON_INTERCEPTOR_WRITE_RANGE(ctx, ptr, size)                    \
   MemoryAccessRange(((TsanInterceptorContext *)ctx)->thr,                 \
                     ((TsanInterceptorContext *)ctx)->pc, (uptr)ptr, size, \
                     true)
 
 #define COMMON_INTERCEPTOR_READ_RANGE(ctx, ptr, size)                       \
   MemoryAccessRange(((TsanInterceptorContext *) ctx)->thr,                  \

compiler-rt/test/sanitizer_common/TestCases/Posix/regex_startend.cpp

+// RUN: %clangxx -O0 -g %s -o %t && %run %t
+/// Check that REG_STARTEND is handled correctly.
+/// This is a regression test for https://github.com/google/sanitizers/issues/1371
+/// Previously, on GLibc systems, the interceptor was calling __compat_regexec
+/// (regexec@GLIBC_2.2.5) insead of the newer __regexec (regexec@GLIBC_2.3.4).
+/// The __compat_regexec strips the REG_STARTEND flag but does not report an error
+/// if other flags are present. This can result in infinite loops for programs that
+/// use REG_STARTEND to find all matches inside a buffer (since ignoring REG_STARTEND
+/// means that the search always starts from the first character).
+
+#include <assert.h>
+#include <regex.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+void test_matched(const regex_t *preg, const char *string, size_t start,
+                  size_t end, const char *expected) {
+  regmatch_t match[1];
+  match[0].rm_so = start;
+  match[0].rm_eo = end;
+  int rv = regexec(preg, string, 1, match, REG_STARTEND);
+  int matchlen = (int)(match[0].rm_eo - match[0].rm_so);
+  const char *matchstart = string + match[0].rm_so;
+  if (rv == 0) {
+    if (expected == nullptr) {
+      fprintf(stderr, "ERROR: expected no match but got '%.*s'\n",
+              matchlen, matchstart);
+      abort();
+    } else if ((size_t)matchlen != strlen(expected) ||
+               memcmp(matchstart, expected, strlen(expected)) != 0) {
+      fprintf(stderr, "ERROR: expected '%s' match but got '%.*s'\n",
+              expected, matchlen, matchstart);
+      abort();
+    }
+  } else if (rv == REG_NOMATCH) {
+    if (expected != nullptr) {
+      fprintf(stderr, "ERROR: expected '%s' match but got no match\n", expected);
+      abort();
+    }
+  } else {
+    printf("ERROR: unexpected regexec return value %d\n", rv);
+    abort();
+  }
+}
+
+int main(void) {
+  regex_t regex;
+  int rv = regcomp(&regex, "[A-Z][A-Z]", 0);
+  assert(!rv);
+  test_matched(&regex, "ABCD", 0, 4, "AB");
+  test_matched(&regex, "ABCD", 0, 1, nullptr); // Not long enough
+  test_matched(&regex, "ABCD", 1, 4, "BC");
+  test_matched(&regex, "ABCD", 1, 2, nullptr); // Not long enough
+  test_matched(&regex, "ABCD", 2, 4, "CD");
+  test_matched(&regex, "ABCD", 2, 3, nullptr); // Not long enough
+  test_matched(&regex, "ABCD", 3, 4, nullptr); // Not long enough
+  regfree(&regex);
+  printf("Successful test\n");
+  return 0;
+}