Fix use-after-free caused by unsync'd access in tcp_client_posix.

tc_on_alarm() and on_writable() race, resulting in the following:

```
D1219 08:59:33.425951347   86323 tcp_client_posix.cc:143]    CLIENT_CONNECT: ipv4:127.0.0.1:27465: on_writable: error="No Error"
D1219 08:59:33.426032150   86342 tcp_client_posix.cc:104]    CLIENT_CONNECT: ipv4:127.0.0.1:27465: on_alarm: error="No Error"
// At this point, note that the callbacks are running on different threads.
D1219 08:59:33.426063521   86323 tcp_client_posix.cc:218]    XXX on_writable ac->addr_str 0x603000008dd0 before unlock. # refs 2->1. Done 0
// on_writable() unrefs while still holding the lock. Because refs > 0, it marks its "done" as false and unlocks.
D1219 08:59:33.426125130   86342 tcp_client_posix.cc:113]    XXX tc_on_alarm ac->addr_str 0x603000008dd0 before unlock. # refs 1->0. Done 1
// right after on_writable() unlocks, tc_on_alarm() acquires the lock and unrefs, this time getting to zero and marking its "done" as true.
// It then proceeds to destroy "ac", and, in particular for this failure, "ac->addr_str".
D1219 08:59:33.426139370   86323 tcp_client_posix.cc:234]    XXX on_writable about to read from ac->addr_str 0x603000008dd0. Done 0, error=OS Error
// When on_writable() tries to read ac->addr_str to assemble its error details, it causes a use-after-free.
```

The problem is the lock isn't held long enough by on_writable(). Alternatively, a copy of ac->addr_str could be made in on_writable() while still holding the lock, but that seems more fragile. It doesn't seem that holding the lock longer would be a performance issue, given we are in a failure scenario.

1 files changed, 1 insertions, 1 deletions

diff --git a/src/core/lib/iomgr/tcp_client_posix.cc b/src/core/lib/iomgr/tcp_client_posix.cc
index 24ccab14b2..799dda5c3e 100644
--- a/src/core/lib/iomgr/tcp_client_posix.cc
+++ b/src/core/lib/iomgr/tcp_client_posix.cc
@@ -212,7 +212,6 @@ finish:
     fd = nullptr;
   }
   done = (--ac->refs == 0);
-  gpr_mu_unlock(&ac->mu);
   if (error != GRPC_ERROR_NONE) {
     char* error_descr;
     grpc_slice str;
@@ -227,6 +226,7 @@ finish:
     error = grpc_error_set_str(error, GRPC_ERROR_STR_TARGET_ADDRESS,
                                grpc_slice_from_copied_string(ac->addr_str));
   }
+  gpr_mu_unlock(&ac->mu);
   if (done) {
     gpr_mu_destroy(&ac->mu);
     gpr_free(ac->addr_str);