Merge "Fix use-after-free in DNS64 discovery thread" into main (b45efd7d) · Commits · e / os / android_packages_modules_DnsResolver

Android.bp

+1 −0

Original line number	Diff line number	Diff line
		@@ -231,6 +231,7 @@ cc_library {
		"libstatslog_resolv",
		"libstatspush_compat",
		"libsysutils",
		"libutils",
		"netd_event_listener_interface-lateststable-ndk",
		"server_configurable_flags",
		"stats_proto",

Dns64Configuration.cpp

+10 −7

Original line number	Diff line number	Diff line
		@@ -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 @@ void Dns64Configuration::startPrefixDiscovery(unsigned netId) {
		// Emplace a copy of \|cfg\| in the map.
		mDns64Configs.emplace(std::make_pair(netId, cfg));

		const sp<Dns64Configuration> thiz = sp<Dns64Configuration>::fromExisting(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 @@ void Dns64Configuration::startPrefixDiscovery(unsigned netId) {
		.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());
		}
		}
		});

Dns64Configuration.h

+2 −1

Original line number	Diff line number	Diff line
		@@ -26,6 +26,7 @@
		#include <android-base/thread_annotations.h>
		#include <netdutils/DumpWriter.h>
		#include <netdutils/InternetAddresses.h>
		#include <utils/RefBase.h>

		struct android_net_context;

		@@ -47,7 +48,7 @@ namespace net {
		* Thread-safety: All public methods in this class MUST be thread-safe.
		* (In other words: this class handles all its locking privately.)
		*/
		class Dns64Configuration {
		class Dns64Configuration : virtual public RefBase {
		public:
		// Simple data struct for passing back packet NAT64 prefix event information to the
		// Dns64PrefixCallback callback.

ResolverController.cpp

+7 −7

Original line number	Diff line number	Diff line
		@@ -155,11 +155,11 @@ int getDnsInfo(unsigned netId, std::vector<std::string>* servers, std::vector<st
		} // namespace

		ResolverController::ResolverController()
		: mDns64Configuration(
		: mDns64Configuration(android::sp<Dns64Configuration>::make(
		[](uint32_t netId, uint32_t uid, android_net_context* netcontext) {
		gResNetdCallbacks.get_network_context(netId, uid, netcontext);
		},
		std::bind(sendNat64PrefixEvent, std::placeholders::_1)) {}
		std::bind(sendNat64PrefixEvent, std::placeholders::_1))) {}

		void ResolverController::destroyNetworkCache(unsigned netId) {
		LOG(VERBOSE) << __func__ << ": netId = " << netId;
		@@ -173,7 +173,7 @@ void ResolverController::destroyNetworkCache(unsigned netId) {
		event.network_type(), event.private_dns_modes(), bytesField);

		resolv_delete_cache_for_net(netId);
		mDns64Configuration.stopPrefixDiscovery(netId);
		mDns64Configuration->stopPrefixDiscovery(netId);
		privateDnsConfiguration.clear(netId);

		// Don't get this instance in PrivateDnsConfiguration. It's probe to deadlock.
		@@ -276,16 +276,16 @@ int ResolverController::getResolverInfo(int32_t netId, std::vector<std::string>*
		}

		void ResolverController::startPrefix64Discovery(int32_t netId) {
		mDns64Configuration.startPrefixDiscovery(netId);
		mDns64Configuration->startPrefixDiscovery(netId);
		}

		void ResolverController::stopPrefix64Discovery(int32_t netId) {
		return mDns64Configuration.stopPrefixDiscovery(netId);
		return mDns64Configuration->stopPrefixDiscovery(netId);
		}

		// TODO: use StatusOr<T> to wrap the result.
		int ResolverController::getPrefix64(unsigned netId, netdutils::IPPrefix* prefix) {
		netdutils::IPPrefix p = mDns64Configuration.getPrefix64(netId);
		netdutils::IPPrefix p = mDns64Configuration->getPrefix64(netId);
		if (p.family() != AF_INET6 \|\| p.length() == 0) {
		return -ENOENT;
		}
		@@ -352,7 +352,7 @@ void ResolverController::dump(DumpWriter& dw, unsigned netId) {
		params.max_samples, params.base_timeout_msec, params.retry_count);
		}

		mDns64Configuration.dump(dw, netId);
		mDns64Configuration->dump(dw, netId);
		const auto privateDnsStatus = PrivateDnsConfiguration::getInstance().getStatus(netId);
		dw.println("Private DNS mode: %s", getPrivateDnsModeString(privateDnsStatus.mode));
		if (privateDnsStatus.dotServersMap.size() == 0) {

ResolverController.h

+3 −3

Original line number	Diff line number	Diff line
		@@ -55,10 +55,10 @@ class ResolverController {

		// Set or clear a NAT64 prefix discovered by other sources (e.g., RA).
		int setPrefix64(unsigned netId, const netdutils::IPPrefix& prefix) {
		return mDns64Configuration.setPrefix64(netId, prefix);
		return mDns64Configuration->setPrefix64(netId, prefix);
		}

		int clearPrefix64(unsigned netId) { return mDns64Configuration.clearPrefix64(netId); }
		int clearPrefix64(unsigned netId) { return mDns64Configuration->clearPrefix64(netId); }

		// Return the current NAT64 prefix network, regardless of how it was discovered.
		int getPrefix64(unsigned netId, netdutils::IPPrefix* prefix);
		@@ -66,7 +66,7 @@ class ResolverController {
		void dump(netdutils::DumpWriter& dw, unsigned netId);

		private:
		Dns64Configuration mDns64Configuration;
		android::sp<Dns64Configuration> mDns64Configuration;
		};
		} // namespace net
		} // namespace android