[sanitizer] Allow getsockname with NULL addrlen

[tamird]

Summary

This is already permitted in getpeername, and returns EFAULT on Linux
(does not crash the program).

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

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

2718–2722

it was a pointer to the int int addr_sz;

3234

can you please add some test, e.g. clone compiler-rt/test/sanitizer_common/TestCases/Linux/recv_msg_trunc.cc and remove unnecessary stuff
in test/sanitizer_common we usually only check that intercepted call does not crash and behaves reasonably.

[tamird]

Add test

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

2718–2722

I don't follow. Are you asking for a change to be made here?

[vitalybuka]

Thanks

[vitalybuka]

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

2718–2722

yes
because is addrlen is "int *"
I assume getsockname will fail for negative number anyway

[tamird]

Fix getsockname prototype

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

2718–2722

The function prototype here is wrong; addrlen is actually socklen_t which is a typedef for unsigned. Fixed.

[tamird]

Stricter test

[tamird]

Could someone land this for me? I don't have commit access.

[vitalybuka]

Crashes on Android https://lab.llvm.org/buildbot/#/builders/77/builds/11119/steps/16/logs/stdio

FAIL: SanitizerCommon-asan-i386-Android :: Linux/get_sock_peer_name.cpp (807 of 1325)
******************** TEST 'SanitizerCommon-asan-i386-Android :: Linux/get_sock_peer_name.cpp' FAILED ********************
Script:
--
: 'RUN: at line 4';     /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/sanitizer_common/android_commands/android_compile.py  /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm_build64/bin/clang  --driver-mode=g++ -gline-tables-only -fsanitize=address  --target=i686-linux-android24 --sysroot=/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot --gcc-toolchain=/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64  -B/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64 -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta -stdlib=libc++ -fuse-ld=lld  -ldl /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/get_sock_peer_name.cpp -o /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_i686/test/sanitizer_common/asan-i386-Android/Linux/Output/get_sock_peer_name.cpp.tmp &&  /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_i686/test/sanitizer_common/asan-i386-Android/Linux/Output/get_sock_peer_name.cpp.tmp 2>&1
--
Exit Code: 134
Command Output (stdout):
--
/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/get_sock_peer_name.cpp:24: int main(): assertion "connect(fd, reinterpret_cast<const sockaddr *>(&sin), sizeof(sin)) == 0" failed
Aborted 
--
Command Output (stderr):
--
+ : 'RUN: at line 4'
+ /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/sanitizer_common/android_commands/android_compile.py /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm_build64/bin/clang --driver-mode=g++ -gline-tables-only -fsanitize=address --target=i686-linux-android24 --sysroot=/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot --gcc-toolchain=/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64 -B/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64 -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta -stdlib=libc++ -fuse-ld=lld -ldl /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/get_sock_peer_name.cpp -o /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_i686/test/sanitizer_common/asan-i386-Android/Linux/Output/get_sock_peer_name.cpp.tmp
+ /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_i686/test/sanitizer_common/asan-i386-Android/Linux/Output/get_sock_peer_name.cpp.tmp
/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_i686/test/sanitizer_common/asan-i386-Android/Linux/Output/get_sock_peer_name.cpp.script: line 1: 14248 Aborted                 (core dumped) /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_i686/test/sanitizer_common/asan-i386-Android/Linux/Output/get_sock_peer_name.cpp.tmp 2>&1
--
********************

[tamird]

Thanks for the fix!

compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc

INTERCEPTOR(int, getnameinfo, void *sockaddr, unsigned salen, char *host,
   return res;
 }
 #define INIT_GETNAMEINFO COMMON_INTERCEPT_FUNCTION(getnameinfo);
 #else
 #define INIT_GETNAMEINFO
 #endif
 
 #if SANITIZER_INTERCEPT_GETSOCKNAME
-INTERCEPTOR(int, getsockname, int sock_fd, void *addr, int *addrlen) {
+INTERCEPTOR(int, getsockname, int sock_fd, void *addr, unsigned *addrlen) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, getsockname, sock_fd, addr, addrlen);
+  unsigned addr_sz;
+  if (addrlen) {
     COMMON_INTERCEPTOR_READ_RANGE(ctx, addrlen, sizeof(*addrlen));
-  int addrlen_in = *addrlen;
+    addr_sz = *addrlen;
+  }
   // FIXME: under ASan the call below may write to freed memory and corrupt
   // its metadata. See
   // https://github.com/google/sanitizers/issues/321.
   int res = REAL(getsockname)(sock_fd, addr, addrlen);
-  if (res == 0) {
-    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, addr, Min(addrlen_in, *addrlen));
+  if (!res && addr && addrlen) {
+    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, addr, Min(addr_sz, *addrlen));
   }
   return res;
 }
 #define INIT_GETSOCKNAME COMMON_INTERCEPT_FUNCTION(getsockname);
 #else
 #define INIT_GETSOCKNAME
 #endif
 

 #define INIT_SYSMSG
 #endif
 
 #if SANITIZER_INTERCEPT_GETPEERNAME
 INTERCEPTOR(int, getpeername, int sockfd, void *addr, unsigned *addrlen) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, getpeername, sockfd, addr, addrlen);
   unsigned addr_sz;
-  if (addrlen) addr_sz = *addrlen;
+  if (addrlen) {
+    COMMON_INTERCEPTOR_READ_RANGE(ctx, addrlen, sizeof(*addrlen));
+    addr_sz = *addrlen;
+  }
   // FIXME: under ASan the call below may write to freed memory and corrupt
   // its metadata. See
   // https://github.com/google/sanitizers/issues/321.
   int res = REAL(getpeername)(sockfd, addr, addrlen);
-  if (!res && addr && addrlen)
+  if (!res && addr && addrlen) {
     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, addr, Min(addr_sz, *addrlen));
+  }
   return res;
 }
 #define INIT_GETPEERNAME COMMON_INTERCEPT_FUNCTION(getpeername);
 #else
 #define INIT_GETPEERNAME
 #endif
 
 #if SANITIZER_INTERCEPT_SYSINFO

compiler-rt/test/sanitizer_common/TestCases/Linux/get_sock_peer_name.cpp

+// Test that ASan doesn't raise false alarm when getsockname and getpeername
+// are called with addrlen=nullptr;
+//
+// RUN: %clangxx %s -o %t && %run %t 2>&1
+
+#include <assert.h>
+#include <errno.h>
+#include <netinet/in.h>
+#include <sys/socket.h>
+
+int main() {
+  const int fd = socket(AF_INET, SOCK_DGRAM, 0);
+  assert(fd >= 0);
+
+  const sockaddr_in sin = {
+      .sin_family = AF_INET,
+      .sin_port = 1234,
+      .sin_addr =
+          {
+              .s_addr = INADDR_LOOPBACK,
+          },
+  };
+  assert(connect(fd, reinterpret_cast<const sockaddr *>(&sin), sizeof(sin)) ==
+         0);
+
+  errno = 0;
+  assert(getsockname(fd, nullptr, nullptr) == -1);
+  assert(errno == EFAULT);
+
+  errno = 0;
+  assert(getpeername(fd, nullptr, nullptr) == -1);
+  assert(errno == EFAULT);
+
+  return 0;
+}