[PATCH] D54078: Implement getpeername(2) interceptor

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:3084
   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)
----------------
krytarowski wrote:
> vitalybuka wrote:
> > Why this and previous versions need temp taddrlen?
> > 
> > 
> In order to handle corner cases when addr / addrlen is passed 0.
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.



================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:3089
+    taddrlen = 0;
+  __sanitizer_sockaddr_storage taddr;
+  int res = REAL(getpeername)(sockfd, addr ? &taddr : 0,
----------------
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


Repository:
  rL LLVM

https://reviews.llvm.org/D54078