From 7b1eb963b1bca487b30c56b7d563aa5493fdf395 Mon Sep 17 00:00:00 2001 From: Mateen Anjum Date: Tue, 30 Jun 2026 13:32:16 -0400 Subject: [PATCH 1/2] redis: fix use-after-free on overlapping cluster discovery refresh CLUSTER SLOTS clears current_request_ as soon as it completes, while the zone-discovery INFO requests it triggers are still in flight. A refresh (periodic resolve timer or DNS update) arriving in that window re-entered startResolveRedis(), started a second zone discovery and overwrote zone_callbacks_, freeing the callbacks that the in-flight INFO requests still referenced. Gate startResolveRedis() on pending_zone_requests_ so a refresh is skipped until zone discovery finishes, and add a regression test. Signed-off-by: Mateen Anjum --- .../redis__zone-discovery-use-after-free.rst | 4 ++ .../clusters/redis/redis_cluster.cc | 11 +++++ .../clusters/redis/redis_cluster_test.cc | 41 +++++++++++++++++++ 3 files changed, 56 insertions(+) create mode 100644 changelogs/current/bug_fixes/redis__zone-discovery-use-after-free.rst diff --git a/changelogs/current/bug_fixes/redis__zone-discovery-use-after-free.rst b/changelogs/current/bug_fixes/redis__zone-discovery-use-after-free.rst new file mode 100644 index 0000000000000..b3ed2c181cc3b --- /dev/null +++ b/changelogs/current/bug_fixes/redis__zone-discovery-use-after-free.rst @@ -0,0 +1,4 @@ +Fixed a use-after-free in the Redis cluster ``CLUSTER SLOTS`` discovery. A cluster refresh +(periodic resolve timer or DNS update) that arrived after ``CLUSTER SLOTS`` completed but while the +zone-discovery ``INFO`` requests it triggered were still in flight could start a second discovery, +overwrite the in-flight callbacks and free memory still referenced by the outstanding requests. diff --git a/source/extensions/clusters/redis/redis_cluster.cc b/source/extensions/clusters/redis/redis_cluster.cc index 7d33baf2d9208..e02f8f12c3d85 100644 --- a/source/extensions/clusters/redis/redis_cluster.cc +++ b/source/extensions/clusters/redis/redis_cluster.cc @@ -364,6 +364,17 @@ void RedisCluster::RedisDiscoverySession::startResolveRedis() { return; } + // current_request_ is cleared as soon as CLUSTER SLOTS completes, while the + // zone-discovery INFO requests it triggers are still in flight. Re-entering + // here in that window would start another zone discovery and overwrite + // zone_callbacks_, freeing the callbacks that the in-flight INFO requests + // still reference (use-after-free). Skip until zone discovery finishes. + if (pending_zone_requests_.load() > 0) { + ENVOY_LOG(debug, "redis cluster zone discovery is already in progress for '{}'", + parent_.info_->name()); + return; + } + // If hosts is empty, we haven't received a successful result from the CLUSTER SLOTS call // yet. So, pick a random discovery address from dns and make a request. Upstream::HostSharedPtr host; diff --git a/test/extensions/clusters/redis/redis_cluster_test.cc b/test/extensions/clusters/redis/redis_cluster_test.cc index 34ca3346cb5b5..0f6f9a4641701 100644 --- a/test/extensions/clusters/redis/redis_cluster_test.cc +++ b/test/extensions/clusters/redis/redis_cluster_test.cc @@ -1801,6 +1801,47 @@ TEST_F(RedisClusterTest, ZoneDiscoveryMakeRequestReturnsNull) { cluster_->prioritySet().hostSetsPerPriority()[0]->hosts()[0]->locality().zone().empty()); } +// A refresh (periodic resolve timer, DNS update) that arrives after CLUSTER +// SLOTS completes but before the zone-discovery INFO replies return must not +// start a second resolution. current_request_ is already null in that window, +// so without gating on pending_zone_requests_ the refresh starts another zone +// discovery and overwrites zone_callbacks_, freeing the callbacks that the +// in-flight INFO requests still reference (use-after-free). +TEST_F(RedisClusterTest, ZoneDiscoveryRefreshWhileInfoInFlightIsSkipped) { + auto* zone_client = setupZoneDiscoveryWithTwoNodes(); + + // Zone discovery is in flight (two INFO requests outstanding). A refresh in + // this window must not issue another CLUSTER SLOTS request. + EXPECT_CALL(*client_, makeRequest_(Ref(RedisCluster::ClusterSlotsRequest::instance_), _)) + .Times(0); + EXPECT_CALL(*zone_client, makeRequest_(Ref(RedisCluster::ClusterSlotsRequest::instance_), _)) + .Times(0); + resolve_timer_->invokeCallback(); + + // The original in-flight INFO callbacks are still valid and complete normally. + NetworkFilters::Common::Redis::RespValuePtr info_resp_1( + new NetworkFilters::Common::Redis::RespValue()); + info_resp_1->type(NetworkFilters::Common::Redis::RespType::BulkString); + info_resp_1->asString() = "# Server\navailability_zone:us-east-1a\n"; + auto zone_cb_1 = std::next(client_->client_callbacks_.begin()); + (*zone_cb_1)->onResponse(std::move(info_resp_1)); + + NetworkFilters::Common::Redis::RespValuePtr info_resp_2( + new NetworkFilters::Common::Redis::RespValue()); + info_resp_2->type(NetworkFilters::Common::Redis::RespType::BulkString); + info_resp_2->asString() = "# Server\navailability_zone:us-east-1b\n"; + zone_client->client_callbacks_.front()->onResponse(std::move(info_resp_2)); + + EXPECT_EQ(2UL, cluster_->prioritySet().hostSetsPerPriority()[0]->hosts().size()); + for (const auto& host : cluster_->prioritySet().hostSetsPerPriority()[0]->hosts()) { + if (host->address()->asString() == "127.0.0.1:22120") { + EXPECT_EQ("us-east-1a", host->locality().zone()); + } else { + EXPECT_EQ("us-east-1b", host->locality().zone()); + } + } +} + } // namespace Redis } // namespace Clusters } // namespace Extensions From c57c8bba5c2abddd0da97ff71b2a7658c17875c6 Mon Sep 17 00:00:00 2001 From: Mateen Anjum Date: Tue, 30 Jun 2026 14:20:55 -0400 Subject: [PATCH 2/2] redis: use canonical redis_proxy changelog area The changelog fragment area must match the canonical set in changelogs/changelogs.yaml; the Redis subsystem area is redis_proxy, not redis. Signed-off-by: Mateen Anjum --- ...er-free.rst => redis_proxy__zone-discovery-use-after-free.rst} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelogs/current/bug_fixes/{redis__zone-discovery-use-after-free.rst => redis_proxy__zone-discovery-use-after-free.rst} (100%) diff --git a/changelogs/current/bug_fixes/redis__zone-discovery-use-after-free.rst b/changelogs/current/bug_fixes/redis_proxy__zone-discovery-use-after-free.rst similarity index 100% rename from changelogs/current/bug_fixes/redis__zone-discovery-use-after-free.rst rename to changelogs/current/bug_fixes/redis_proxy__zone-discovery-use-after-free.rst