Fix use-after-free in DNS64 discovery thread
DNS64 discovery thread is detached from binder requesting thread. But
the discovery thread references resources not belongs to itself, which
can be destroyed in dnsresolver destruction.
Holds a strong pointer of Dns64Configuration in DNS64 discovery thread
so that the instance of Dns64Configuration will keep until the DNS64
thread is force terminated.
Ignore-AOSP-First: Fix security vulnerability
Bug: 278303745
Test: m, fuzzing
Fuzzing: mma resolv_service_fuzzer && adb sync data && adb shell /data/fuzz/arm64/resolv_service_fuzzer/resolv_service_fuzzer
(cherry picked from commit 254115584ff558fb87ee6ec5f5bb043f76219910)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:79f571069c4db536e7c1bfecbb50c926f0ef0548)
Merged-In: Id74ea4e6f54a00805d3cc8a9d7e15e58a473b7d3
Change-Id: Id74ea4e6f54a00805d3cc8a9d7e15e58a473b7d3
(cherry picked from commit 9f20f38658ea8027a82f2b9e7946cafaeff69885)
(cherry picked from commit 5716833e9c63a8385163ff03e7726820226519ff)
diff --git a/Dns64Configuration.cpp b/Dns64Configuration.cpp
index a1fe871..6e5ca5b 100644
--- a/Dns64Configuration.cpp
+++ b/Dns64Configuration.cpp
@@ -24,6 +24,7 @@
#include <netdutils/DumpWriter.h>
#include <netdutils/InternetAddresses.h>
#include <netdutils/ThreadUtil.h>
+#include <utils/StrongPointer.h>
#include <thread>
#include <utility>
@@ -36,6 +37,7 @@
namespace android {
+using android::sp;
using netdutils::DumpWriter;
using netdutils::IPAddress;
using netdutils::IPPrefix;
@@ -61,8 +63,9 @@
// Emplace a copy of |cfg| in the map.
mDns64Configs.emplace(std::make_pair(netId, cfg));
+ const sp<Dns64Configuration> thiz = this;
// Note that capturing |cfg| in this lambda creates a copy.
- std::thread discovery_thread([this, cfg, netId] {
+ std::thread discovery_thread([thiz, cfg, netId] {
setThreadName(fmt::format("Nat64Pfx_{}", netId));
// Make a mutable copy rather than mark the whole lambda mutable.
@@ -75,28 +78,28 @@
.build();
while (true) {
- if (!this->shouldContinueDiscovery(evalCfg)) break;
+ if (!thiz->shouldContinueDiscovery(evalCfg)) break;
android_net_context netcontext{};
- mGetNetworkContextCallback(evalCfg.netId, 0, &netcontext);
+ thiz->mGetNetworkContextCallback(evalCfg.netId, 0, &netcontext);
// Prefix discovery must bypass private DNS because in strict mode
// the server generally won't know the NAT64 prefix.
netcontext.flags |= NET_CONTEXT_FLAG_USE_LOCAL_NAMESERVERS;
if (doRfc7050PrefixDiscovery(netcontext, &evalCfg)) {
- this->recordDns64Config(evalCfg);
+ thiz->recordDns64Config(evalCfg);
break;
}
- if (!this->shouldContinueDiscovery(evalCfg)) break;
+ if (!thiz->shouldContinueDiscovery(evalCfg)) break;
if (!backoff.hasNextTimeout()) break;
{
- std::unique_lock<std::mutex> cvGuard(mMutex);
+ std::unique_lock<std::mutex> cvGuard(thiz->mMutex);
// TODO: Consider some chrono math, combined with wait_until()
// perhaps, to prevent early re-resolves from the removal of
// other netids with IPv6-only nameservers.
- mCv.wait_for(cvGuard, backoff.getNextTimeout());
+ thiz->mCv.wait_for(cvGuard, backoff.getNextTimeout());
}
}
});