From b23c7e608fe71898fc48ae85e019f7eadd890ad1 Mon Sep 17 00:00:00 2001
From: Kenichi Ishibashi <[email protected]>
Date: Mon, 01 Jun 2026 18:35:17 -0700
Subject: [PATCH] net: Fix potential teardown crash in HostResolverManager

During ResolveContext deregistration, HostResolverManager::RemoveAllJobs
cancels and removes all active jobs associated with the context.
If the destruction of a job (such as an outer NAT64 job) synchronously
triggers the cancellation and removal of a nested job (such as the inner
AAAA job for ipv4only.arpa), the iteration state in RemoveAllJobs could
be invalidated because the nested job erases itself from the jobs_ map
while it is being iterated.

This CL fixes the issue by collecting the Job objects to be removed in a
temporary vector during the iteration loop, and then explicitly
destroying them after the iteration is complete. This ensures that any
synchronous side-effects of job destruction do not interfere with the
map iteration.

Bug: 513714124
Change-Id: I7236ad5c3ac03c0fed0c6c99f81cdbae281f9165
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7881379
Reviewed-by: Adam Rice <[email protected]>
Commit-Queue: Kenichi Ishibashi <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1639847}
---

diff --git a/net/dns/host_resolver_manager.cc b/net/dns/host_resolver_manager.cc
index 20df619b..e583868 100644
--- a/net/dns/host_resolver_manager.cc
+++ b/net/dns/host_resolver_manager.cc
@@ -1638,14 +1638,20 @@
 }
 
 void HostResolverManager::RemoveAllJobs(const ResolveContext* context) {
+  // Job destructor can re-enter jobs_.erase() (e.g., via HostResolverNat64Task
+  // destructor destroying a nested RequestImpl whose CancelRequest
+  // synchronously removes a different Job). Collect the Jobs first and destroy
+  // them after the iteration loop to prevent iterator invalidation.
+  std::vector<std::unique_ptr<Job>> jobs_to_destroy;
   for (auto it = jobs_.begin(); it != jobs_.end();) {
     const JobKey& key = it->first;
     if (&*key.resolve_context == context) {
-      RemoveJob(it++);
+      jobs_to_destroy.push_back(RemoveJob(it++));
     } else {
       ++it;
     }
   }
+  jobs_to_destroy.clear();
 }
 
 void HostResolverManager::AbortJobsWithoutTargetNetwork(bool in_progress_only) {
diff --git a/net/dns/host_resolver_manager_unittest.cc b/net/dns/host_resolver_manager_unittest.cc
index 7d38cb9..300ade3 100644
--- a/net/dns/host_resolver_manager_unittest.cc
+++ b/net/dns/host_resolver_manager_unittest.cc
@@ -14574,6 +14574,49 @@
   IPv4AddressLiteralInIPv6OnlyNetworkBadAddressTest(false);
 }
 
+// Regression test for crbug.com/513714124.
+//
+// When a ResolveContext is deregistered, all active jobs associated with it
+// are cancelled. For some requests (like NAT64 translation), a job may own
+// a nested request that is attached to a different job. If cancelling the
+// outer job synchronously cancels and removes the nested job, we must ensure
+// this nested removal does not invalidate the iteration state used to clean
+// up the remaining jobs.
+TEST_F(HostResolverManagerTest,
+       Nat64DeregisterContextDoesNotInvalidateIterator) {
+  HostResolver::ManagerOptions options = DefaultOptions();
+  CreateResolverWithOptionsAndParams(std::move(options), DefaultParams(proc_),
+                                     /*ipv6_reachable=*/true,
+                                     /*is_async=*/false,
+                                     /*ipv4_reachable=*/false);
+  proc_->AddRule("ipv4only.arpa", ADDRESS_FAMILY_IPV6,
+                 "64:ff9b::c000:aa,64:ff9b::c000:ab");
+
+  HostResolver::ResolveHostParameters params;
+  params.dns_query_type = DnsQueryType::A;
+  ResolveHostResponseHelper response(resolver_->CreateRequest(
+      HostPortPair("192.168.1.42", 80), NetworkAnonymizationKey(),
+      NetLogWithSource(), params, resolve_context_.get()));
+
+  ASSERT_FALSE(response.complete());
+  // Wait for the inner ipv4only.arpa system task's worker to block so both
+  // the outer NAT64 Job ({A}) and the inner Job ({AAAA}) are live in jobs_.
+  ASSERT_TRUE(proc_->WaitFor(1u));
+  ASSERT_EQ(2u, resolver_->num_jobs_for_testing());
+
+  // ResolveContext teardown while the inner ipv4only.arpa lookup is in
+  // flight. Without the fix this is a heap-use-after-free under ASan.
+  resolver_->DeregisterResolveContext(resolve_context_.get());
+
+  EXPECT_EQ(0u, resolver_->num_jobs_for_testing());
+
+  // Cleanup: release the blocked worker and re-register the context so
+  // TearDown's DeregisterResolveContext is balanced.
+  proc_->SignalAll();
+  base::ThreadPoolInstance::Get()->FlushForTesting();
+  resolver_->RegisterResolveContext(resolve_context_.get());
+}
+
 TEST_F(HostResolverManagerDnsTest, ResolutionDetails_InsecureDnsSuccess) {
   ChangeDnsConfig(CreateValidDnsConfig());
 

diff --git a/net/dns/host_resolver_manager_unittest.cc b/net/dns/host_resolver_manager_unittest.cc
index 7d38cb9..300ade3 100644
--- a/net/dns/host_resolver_manager_unittest.cc
+++ b/net/dns/host_resolver_manager_unittest.cc
@@ -14574,6 +14574,49 @@
   IPv4AddressLiteralInIPv6OnlyNetworkBadAddressTest(false);
 }
 
+// Regression test for crbug.com/513714124.
+//
+// When a ResolveContext is deregistered, all active jobs associated with it
+// are cancelled. For some requests (like NAT64 translation), a job may own
+// a nested request that is attached to a different job. If cancelling the
+// outer job synchronously cancels and removes the nested job, we must ensure
+// this nested removal does not invalidate the iteration state used to clean
+// up the remaining jobs.
+TEST_F(HostResolverManagerTest,
+       Nat64DeregisterContextDoesNotInvalidateIterator) {
+  HostResolver::ManagerOptions options = DefaultOptions();
+  CreateResolverWithOptionsAndParams(std::move(options), DefaultParams(proc_),
+                                     /*ipv6_reachable=*/true,
+                                     /*is_async=*/false,
+                                     /*ipv4_reachable=*/false);
+  proc_->AddRule("ipv4only.arpa", ADDRESS_FAMILY_IPV6,
+                 "64:ff9b::c000:aa,64:ff9b::c000:ab");
+
+  HostResolver::ResolveHostParameters params;
+  params.dns_query_type = DnsQueryType::A;
+  ResolveHostResponseHelper response(resolver_->CreateRequest(
+      HostPortPair("192.168.1.42", 80), NetworkAnonymizationKey(),
+      NetLogWithSource(), params, resolve_context_.get()));
+
+  ASSERT_FALSE(response.complete());
+  // Wait for the inner ipv4only.arpa system task's worker to block so both
+  // the outer NAT64 Job ({A}) and the inner Job ({AAAA}) are live in jobs_.
+  ASSERT_TRUE(proc_->WaitFor(1u));
+  ASSERT_EQ(2u, resolver_->num_jobs_for_testing());
+
+  // ResolveContext teardown while the inner ipv4only.arpa lookup is in
+  // flight. Without the fix this is a heap-use-after-free under ASan.
+  resolver_->DeregisterResolveContext(resolve_context_.get());
+
+  EXPECT_EQ(0u, resolver_->num_jobs_for_testing());
+
+  // Cleanup: release the blocked worker and re-register the context so
+  // TearDown's DeregisterResolveContext is balanced.
+  proc_->SignalAll();
+  base::ThreadPoolInstance::Get()->FlushForTesting();
+  resolver_->RegisterResolveContext(resolve_context_.get());
+}
+
 TEST_F(HostResolverManagerDnsTest, ResolutionDetails_InsecureDnsSuccess) {
   ChangeDnsConfig(CreateValidDnsConfig());