Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 93d3a164 authored by Ken Chen's avatar Ken Chen Committed by Automerger Merge Worker
Browse files

Fix use-after-free in DNS64 discovery thread am: 292b59e1 

Original change: https://googleplex-android-review.googlesource.com/c/platform/packages/modules/DnsResolver/+/24557576



Change-Id: I14e9ee228dde02f99dc514be9dbe85fa40c95554
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents a8b0eec0 292b59e1

Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -201,6 +201,7 @@ cc_library { 
        "libstatspush_compat",

        "libsysutils",

        "netd_event_listener_interface-lateststable-ndk_platform",

        "libutils",

        "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>
@@ -38,6 +39,7 @@ namespace android { 


using android::base::StringPrintf;

using android::net::NetworkDnsEventReported;

using android::sp;

using netdutils::DumpWriter;

using netdutils::IPAddress;

using netdutils::IPPrefix;
@@ -63,8 +65,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 = 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(StringPrintf("Nat64Pfx_%u", netId).c_str());



        // Make a mutable copy rather than mark the whole lambda mutable.
@@ -77,28 +80,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 −8
Original line number Diff line number Diff line
@@ -155,17 +155,17 @@ int getDnsInfo(unsigned netId, std::vector<std::string>* servers, std::vector<st 
}  // namespace



ResolverController::ResolverController()

    : mDns64Configuration(

    : mDns64Configuration(new Dns64Configuration(

              [](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;



    resolv_delete_cache_for_net(netId);

    mDns64Configuration.stopPrefixDiscovery(netId);

    mDns64Configuration->stopPrefixDiscovery(netId);

    PrivateDnsConfiguration::getInstance().clear(netId);



    // Don't get this instance in PrivateDnsConfiguration. It's probe to deadlock.
@@ -263,16 +263,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) {

        LOG(INFO) << "No valid NAT64 prefix (" << netId << ", " << p.toString().c_str() << ")";
@@ -338,8 +338,7 @@ void ResolverController::dump(DumpWriter& dw, unsigned netId) { 
                    params.sample_validity, params.success_threshold, params.min_samples,

                    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.serversMap.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