[tsan] Refine fstat{,64} interceptors on GLIBC

[wangleiat]

Without this patch, DEADLYSIGNAL will occur on some architectures.

ThreadSanitizer:DEADLYSIGNAL
==752233==ERROR: ThreadSanitizer: SEGV on unknown address 0x000000000000 (pc 0x000000000000 bp 0x7ffffbdfe080 sp 0x7ffffbdfe010 T752233)
==752233==Hint: pc points to the zero page.
==752233==The signal is caused by a READ memory access.
==752233==Hint: address points to the zero page.

The reason is that ports introduced in Glibc 2.33 and later will not have the opportunity to implement the __fxstat{,64} interfaces.
For example:

• LoongArch - Glibc version 2.36

GLIBC_2.36 fstat F
GLIBC_2.36 fstat64 F

• RISC-V variants (RISCV_ABI_XLEN == 32 && (RISCV_ABI_FLEN == 64 || RISCV_ABI_FLEN == 0) - Glibc version 2.33

GLIBC_2.33 fstat F
GLIBC_2.33 fstat64 F

[llvmbot]

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

Author: wanglei (wangleiat)

Changes

Without this patch, use-of-uninitialized value will occur. Because those architectures, which are supported by Glibc after version 2.33, will be no opportunity to implement the __fxstat{,64} interfaces.

---

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

1 Files Affected:

• (modified) compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp (+8-1)

diff --git a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
index 2bebe651b994e5..cbdd6d9ab44158 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
@@ -1620,7 +1620,7 @@ TSAN_INTERCEPTOR(int, __fxstat, int version, int fd, void *buf) {
 #endif
 
 TSAN_INTERCEPTOR(int, fstat, int fd, void *buf) {
-#if SANITIZER_GLIBC
+#if SANITIZER_GLIBC && !__GLIBC_PREREQ(2, 33)
   SCOPED_TSAN_INTERCEPTOR(__fxstat, 0, fd, buf);
   if (fd > 0)
     FdAccess(thr, pc, fd);
@@ -1647,10 +1647,17 @@ TSAN_INTERCEPTOR(int, __fxstat64, int version, int fd, void *buf) {
 
 #if SANITIZER_GLIBC
 TSAN_INTERCEPTOR(int, fstat64, int fd, void *buf) {
+#  if !__GLIBC_PREREQ(2, 33)
   SCOPED_TSAN_INTERCEPTOR(__fxstat64, 0, fd, buf);
   if (fd > 0)
     FdAccess(thr, pc, fd);
   return REAL(__fxstat64)(0, fd, buf);
+#  else
+  SCOPED_TSAN_INTERCEPTOR(fstat64, fd, buf);
+  if (fd > 0)
+    FdAccess(thr, pc, fd);
+  return REAL(fstat64)(fd, buf);
+#  endif
 }
 #define TSAN_MAYBE_INTERCEPT_FSTAT64 TSAN_INTERCEPT(fstat64)
 #else

[SixWeining]

Add @lixing-star who reported this issue to us.

[MaskRay]

Without this patch, use-of-uninitialized value will occur on those architectures, which are supported by Glibc since version 2.33. Because they have no opportunity to implement the __fxstat{,64} interfaces.

"those architectures" are not clear. I assume they include LoongArch (base version 2.36) and some RISC-V variants (sysdeps/unix/sysv/linux/riscv/shlib-versions)

This description needs rewording. I think there may be a difference if a glibc port has a base version >= 2.33. https://maskray.me/blog/2022-05-29-glibc#buildabi-versions.h

Can you describe what f*stat symbols your glibc port defines?

[vitalybuka]

use-of-uninitialized is MSAN, but the patch is TSAN only?

[wangleiat]

Without this patch, use-of-uninitialized value will occur on those architectures, which are supported by Glibc since version 2.33. Because they have no opportunity to implement the __fxstat{,64} interfaces.

"those architectures" are not clear. I assume they include LoongArch (base version 2.36) and some RISC-V variants (sysdeps/unix/sysv/linux/riscv/shlib-versions)

This description needs rewording. I think there may be a difference if a glibc port has a base version >= 2.33. https://maskray.me/blog/2022-05-29-glibc#buildabi-versions.h

Can you describe what f*stat symbols your glibc port defines?

Thank you very much for your blog; I've learned a lot from it. I've attempted to modify the commit message using my limited English vocabulary.

[wangleiat]

use-of-uninitialized is MSAN, but the patch is TSAN only?

I apologize for my inadequate English description causing inconvenience. My intention was that on systems where __fxstat{,64} is not implemented, interception would result in calling an empty function.

MSAN does not have this issue.

V#define SANITIZER_STAT_LINUX (SANITIZER_LINUX && __GLIBC_PREREQ(2, 33)) 
#if SANITIZER_FREEBSD || SANITIZER_NETBSD || SANITIZER_STAT_LINUX       
INTERCEPTOR(int, fstat, int fd, void *buf) {                            
  ENSURE_MSAN_INITED();                                                 
  int res = REAL(fstat)(fd, buf);                                       
  if (!res)                                                             
    __msan_unpoison(buf, __sanitizer::struct_stat_sz);                  
  return res;                                                           
}                                                                       
#  define MSAN_MAYBE_INTERCEPT_FSTAT MSAN_INTERCEPT_FUNC(fstat)         
#else                                                                   
#define MSAN_MAYBE_INTERCEPT_FSTAT                                      
#endif                                                                  
                                                                        
#if SANITIZER_STAT_LINUX                                                
INTERCEPTOR(int, fstat64, int fd, void *buf) {                          
  ENSURE_MSAN_INITED();                                                 
  int res = REAL(fstat64)(fd, buf);                                     
  if (!res)                                                             
    __msan_unpoison(buf, __sanitizer::struct_stat64_sz);                
  return res;                                                           
}                                                                       
#  define MSAN_MAYBE_INTERCEPT_FSTAT64 MSAN_INTERCEPT_FUNC(fstat64)     
#else                                                                   
#  define MSAN_MAYBE_INTERCEPT_FSTAT64                                  
#endif                                                                  

[wangleiat]

Add @lixing-star who reported this issue to us.

Add @abner-chenc The original issue discoverer.

[MaskRay]

I think we should refine the interceptor conditions and use the correct symbols. I am writing a patch to properly fix this.

[MaskRay]

The interceptors should now use __fxstat{,64} compatibility symbols. Sent #86625 to additionally simplify the code a bit.