From 2c9e127ac9bb39156c0509560b162f24fd37fd12 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 13 Jun 2024 10:44:28 -0700 Subject: [PATCH] [chaotic-good] Fix use-after-free (#36826) Previously AbortWithError could race with destruction Built on #36825 which should be merged first Closes #36826 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36826 from ctiller:csc2 e79c87984536c8e36280cf07c2e771723324b2c8 PiperOrigin-RevId: 643050342 --- .../chaotic_good/client_transport.cc | 19 ++++++++++++++++++- .../transport/chaotic_good/client_transport.h | 5 +---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/core/ext/transport/chaotic_good/client_transport.cc b/src/core/ext/transport/chaotic_good/client_transport.cc index ddcdaa12ead..5d68d93cf64 100644 --- a/src/core/ext/transport/chaotic_good/client_transport.cc +++ b/src/core/ext/transport/chaotic_good/client_transport.cc @@ -56,6 +56,21 @@ namespace grpc_core { namespace chaotic_good { +void ChaoticGoodClientTransport::Orphan() { + LOG(INFO) << "ChaoticGoodClientTransport::Orphan"; + AbortWithError(); + ActivityPtr writer; + ActivityPtr reader; + { + MutexLock lock(&mu_); + writer = std::move(writer_); + reader = std::move(reader_); + } + writer.reset(); + reader.reset(); + Unref(); +} + auto ChaoticGoodClientTransport::TransportWriteLoop( RefCountedPtr transport) { return Loop([this, transport = std::move(transport)] { @@ -176,7 +191,9 @@ auto ChaoticGoodClientTransport::TransportReadLoop( } auto ChaoticGoodClientTransport::OnTransportActivityDone() { - return [this](absl::Status) { AbortWithError(); }; + return [self = RefAsSubclass()](absl::Status) { + self->AbortWithError(); + }; } ChaoticGoodClientTransport::ChaoticGoodClientTransport( diff --git a/src/core/ext/transport/chaotic_good/client_transport.h b/src/core/ext/transport/chaotic_good/client_transport.h index 12a96403d9a..de37b8d7952 100644 --- a/src/core/ext/transport/chaotic_good/client_transport.h +++ b/src/core/ext/transport/chaotic_good/client_transport.h @@ -82,10 +82,7 @@ class ChaoticGoodClientTransport final : public ClientTransport { void SetPollset(grpc_stream*, grpc_pollset*) override {} void SetPollsetSet(grpc_stream*, grpc_pollset_set*) override {} void PerformOp(grpc_transport_op*) override; - void Orphan() override { - AbortWithError(); - Unref(); - } + void Orphan() override; void StartCall(CallHandler call_handler) override; void AbortWithError();