Implement getpeername(2) interceptor

[krytarowski]

Summary

The previous version was incomplete.

This fixes issues detected on NetBSD.

[krytarowski]

ping

[vitalybuka]

Could you please fix TEST_P(MemorySanitizerIpTest, accept) so that old version fail on poisoned addrlen?

lib/sanitizer_common/sanitizer_common_interceptors.inc

3084

Why this and previous versions need temp taddrlen?

3090

should we

if (addr)
  COMMON_INTERCEPTOR_READ_RANGE(ctx, addrlen, sizeof(*addrlen));

before calling REAL

[vitalybuka]

lib/sanitizer_common/sanitizer_common_interceptors.inc

3090

if (addr && addrlen)
  COMMON_INTERCEPTOR_READ_RANGE(ctx, addrlen, sizeof(*addrlen));

[krytarowski]

lib/sanitizer_common/sanitizer_common_interceptors.inc

3084

In order to handle corner cases when addr / addrlen is passed 0.

[vitalybuka]

lib/sanitizer_common/sanitizer_common_interceptors.inc

3084

Oh, previous code used addr_sz just to store old value to do MIN().
Now behavior is very different.
Also it's not clear from man, but addrlen must be != null, or it fail or crash.

3089

Looks like you are trying to fix https://github.com/google/sanitizers/issues/321, but description does not cover it.
so what if sizeoff(__sanitizer_sockaddr_storage) < *addrlen

[vitalybuka]

lib/sanitizer_common/sanitizer_common_interceptors.inc

3089

maybe you should just ignore 321 for now as it's more general issue, keep FIXME and remove temps.

so you can make it complete by adding "READ_RANGE(ctx, addrlen" and "WRITE_RANGE(ctx, addrlen"

3093

if (addrlen) {
  COMMON_INTERCEPTOR_WRITE_RANGE(ctx, addrlen, sizeof(*addrlen));
   if (addr)
     COMMON_INTERCEPTOR_WRITE_RANGE(ctx, addr, *addrlen);
}

[krytarowski]

I'm confused with this one... unless someone will fix it earlier (maybe taking inspiration from this diff) - it's rescheduled for future on my side.

lib/sanitizer_common/sanitizer_common_interceptors.inc

 #endif
 
 #if SANITIZER_INTERCEPT_GETPEERNAME
-INTERCEPTOR(int, getpeername, int sockfd, void *addr, unsigned *addrlen) {
+INTERCEPTOR(int, getpeername, int sockfd, void *addr,
+  __sanitizer_socklen_t *addrlen) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER(ctx, getpeername, sockfd, addr, addrlen);
-  unsigned addr_sz;
-  if (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)
-    COMMON_INTERCEPTOR_WRITE_RANGE(ctx, addr, Min(addr_sz, *addrlen));
+  __sanitizer_socklen_t taddrlen;
+  if (addrlen)
+    taddrlen = *addrlen;
+  else
+    taddrlen = 0;
+  __sanitizer_sockaddr_storage taddr;
+  int res = REAL(getpeername)(sockfd, addr ? &taddr : 0,
+                              addrlen ? &taddrlen : 0);
+  if (!res) {
+    if (addr && addrlen)
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, addr, taddrlen);
+    if (addrlen)
+      COMMON_INTERCEPTOR_WRITE_RANGE(ctx, addrlen, sizeof(*addrlen));
+  }
+  if (addr && addrlen)
+    memcpy(addr, &taddr, taddrlen);
+  if (addrlen)
+    *addrlen = taddrlen;
   return res;
 }
 #define INIT_GETPEERNAME COMMON_INTERCEPT_FUNCTION(getpeername);