Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dfsan] Make sprintf interceptor compatible with glibc 2.37+ and musl #78363

Conversation

MaskRay
Copy link
Member

@MaskRay MaskRay commented Jan 16, 2024

snprintf interceptors call format_buffer with size==~0ul, which
may eventually lead to snprintf(s, n, "Hello world!") where s+n
wraps around. Since glibc 2.37 (https://sourceware.org/PR30441), the
snprintf call does not write the last char. musl snprintf returns -1
with EOVERFLOW when n > INT_MAX.

Change size to INT_MAX to work with glibc 2.37+ and musl.
snprintf interceptors are not changed. It's user responsibility to not
cause a compatibility issue with libc implementations.

Fix #60678

Created using spr 1.3.4
@llvmbot
Copy link
Member

llvmbot commented Jan 16, 2024

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

Author: Fangrui Song (MaskRay)

Changes

snprintf interceptors call format_buffer with size==~0ul, which
may eventually lead to snprintf(s, n, "Hello world!") where s+n
wraps around. Since glibc 2.37 (https://sourceware.org/PR31251), the
snprintf call does not write the last char. musl snprintf returns -1
with EOVERFLOW when n > INT_MAX.

Change size to INT_MAX to work with glibc 2.37+ and musl.
snprintf interceptors are not changed. It's user responsibility to not
cause a compatibility issue with libc implementations.

Fix #60678


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

3 Files Affected:

  • (modified) compiler-rt/lib/dfsan/dfsan_custom.cpp (+3-3)
  • (modified) compiler-rt/test/dfsan/custom.cpp (-3)
  • (modified) compiler-rt/test/dfsan/release_shadow_space.c (-3)
diff --git a/compiler-rt/lib/dfsan/dfsan_custom.cpp b/compiler-rt/lib/dfsan/dfsan_custom.cpp
index 05b48fd0525e335..c5c14a2d1b0eb43 100644
--- a/compiler-rt/lib/dfsan/dfsan_custom.cpp
+++ b/compiler-rt/lib/dfsan/dfsan_custom.cpp
@@ -2792,7 +2792,7 @@ int __dfsw_sprintf(char *str, const char *format, dfsan_label str_label,
   va_list ap;
   va_start(ap, ret_label);
 
-  int ret = format_buffer(str, ~0ul, format, va_labels, ret_label, nullptr,
+  int ret = format_buffer(str, INT32_MAX, format, va_labels, ret_label, nullptr,
                           nullptr, ap);
   va_end(ap);
   return ret;
@@ -2806,8 +2806,8 @@ int __dfso_sprintf(char *str, const char *format, dfsan_label str_label,
                    dfsan_origin *ret_origin, ...) {
   va_list ap;
   va_start(ap, ret_origin);
-  int ret = format_buffer(str, ~0ul, format, va_labels, ret_label, va_origins,
-                          ret_origin, ap);
+  int ret = format_buffer(str, INT32_MAX, format, va_labels, ret_label,
+                          va_origins, ret_origin, ap);
   va_end(ap);
   return ret;
 }
diff --git a/compiler-rt/test/dfsan/custom.cpp b/compiler-rt/test/dfsan/custom.cpp
index 56e66fd5069a682..2ebeb1e4519782d 100644
--- a/compiler-rt/test/dfsan/custom.cpp
+++ b/compiler-rt/test/dfsan/custom.cpp
@@ -1,6 +1,3 @@
-// https://github.com/llvm/llvm-project/issues/60678
-// XFAIL: glibc-2.37
-
 // RUN: %clang_dfsan %s -o %t && DFSAN_OPTIONS="strict_data_dependencies=0" %run %t
 // RUN: %clang_dfsan -DSTRICT_DATA_DEPENDENCIES %s -o %t && %run %t
 // RUN: %clang_dfsan -DORIGIN_TRACKING -mllvm -dfsan-track-origins=1 -mllvm -dfsan-combine-pointer-labels-on-load=false -DSTRICT_DATA_DEPENDENCIES %s -o %t && %run %t
diff --git a/compiler-rt/test/dfsan/release_shadow_space.c b/compiler-rt/test/dfsan/release_shadow_space.c
index 9492f2ad7728ccf..675640a1c296da2 100644
--- a/compiler-rt/test/dfsan/release_shadow_space.c
+++ b/compiler-rt/test/dfsan/release_shadow_space.c
@@ -1,6 +1,3 @@
-// https://github.com/llvm/llvm-project/issues/60678
-// XFAIL: glibc-2.37
-
 // DFSAN_OPTIONS=no_huge_pages_for_shadow=false RUN: %clang_dfsan %s -o %t && %run %t
 // DFSAN_OPTIONS=no_huge_pages_for_shadow=true RUN: %clang_dfsan %s -o %t && %run %t
 // DFSAN_OPTIONS=no_huge_pages_for_shadow=false RUN: %clang_dfsan %s -DORIGIN_TRACKING -mllvm -dfsan-track-origins=1 -o %t && %run %t

@MaskRay MaskRay merged commit 67e0f41 into main Jan 18, 2024
5 of 6 checks passed
@MaskRay MaskRay deleted the users/MaskRay/spr/dfsan-make-sprintf-interceptor-compatible-with-glibc-237-and-musl branch January 18, 2024 01:14
ampandey-1995 pushed a commit to ampandey-1995/llvm-project that referenced this pull request Jan 19, 2024
…llvm#78363)

snprintf interceptors call `format_buffer` with `size==~0ul`, which
may eventually lead to `snprintf(s, n, "Hello world!")` where `s+n`
wraps around. Since glibc 2.37 (https://sourceware.org/PR30441), the
snprintf call does not write the last char. musl snprintf returns -1
with EOVERFLOW when `n > INT_MAX`.

Change `size` to INT_MAX to work with glibc 2.37+ and musl.
snprintf interceptors are not changed. It's user responsibility to not
cause a compatibility issue with libc implementations.

Fix llvm#60678
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.

DFSAN test failures with glibc 2.37
3 participants