Fix use after free (#28840)

Previously we'd use an explicit arena->Destroy() call to free memory.
This change makes the arena a scoped pointer, and in doing so lets the
grpc_metadata_batch destructors run prior to the arena being destroyed,
preventing a use-after-free we've seen in production code.
pull/28827/head^2
Craig Tiller 3 years ago committed by GitHub
parent b458db9246
commit 66cf5ea6e0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 11
      src/core/ext/filters/client_channel/health/health_check_client.cc
  2. 2
      src/core/ext/filters/client_channel/health/health_check_client.h

@ -262,10 +262,10 @@ HealthCheckClient::CallState::CallState(
->GetInitialCallSizeEstimate(),
&health_check_client_->call_allocator_)),
payload_(context_),
send_initial_metadata_(arena_),
send_trailing_metadata_(arena_),
recv_initial_metadata_(arena_),
recv_trailing_metadata_(arena_) {}
send_initial_metadata_(arena_.get()),
send_trailing_metadata_(arena_.get()),
recv_initial_metadata_(arena_.get()),
recv_trailing_metadata_(arena_.get()) {}
HealthCheckClient::CallState::~CallState() {
if (GRPC_TRACE_FLAG_ENABLED(grpc_health_check_client_trace)) {
@ -282,7 +282,6 @@ HealthCheckClient::CallState::~CallState() {
// any, so that it can release any internal references it may be
// holding to the call stack.
call_combiner_.SetNotifyOnCancel(nullptr);
arena_->Destroy();
}
void HealthCheckClient::CallState::Orphan() {
@ -297,7 +296,7 @@ void HealthCheckClient::CallState::StartCall() {
Slice::FromStaticString("/grpc.health.v1.Health/Watch"),
gpr_get_cycle_counter(), // start_time
GRPC_MILLIS_INF_FUTURE, // deadline
arena_,
arena_.get(),
context_,
&call_combiner_,
};

@ -93,7 +93,7 @@ class HealthCheckClient : public InternallyRefCounted<HealthCheckClient> {
RefCountedPtr<HealthCheckClient> health_check_client_;
grpc_polling_entity pollent_;
Arena* arena_;
ScopedArenaPtr arena_;
CallCombiner call_combiner_;
grpc_call_context_element context_[GRPC_CONTEXT_COUNT] = {};

Loading…
Cancel
Save