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

Commit 021c3e1e authored by Mike Yu's avatar Mike Yu
Browse files

Replace addrinfo with IPSockAddr to store dns addresses 

The struct addrinfo is designed to store multi-addresses and it
might cause memory leaks if improperly used. IPSockAddr is safer
and is extensible. It also helps simplify the struct resolv_cache_info,
where nameservers and nscount is no longer necessary.

Bug: 130686826
Test: atest --include-subdirs packages/modules/DnsResolver
Change-Id: I3243f2f79c94ebe3d03503914d25b5863da20c09
parent e655b1d8

Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -51,6 +51,7 @@ cc_library { 
        "res_query.cpp",

        "res_send.cpp",

        "res_stats.cpp",

        "util.cpp",

        "Dns64Configuration.cpp",

        "DnsProxyListener.cpp",

        "DnsResolver.cpp",

getaddrinfo.cpp

+1 −1
Original line number Diff line number Diff line
@@ -1720,7 +1720,7 @@ static int res_searchN(const char* name, res_target* target, res_state res, int* 
         * the domain stuff is tried.  Will have a better

         * fix after thread pools are used.

         */

        _resolv_populate_res_for_net(res);

        resolv_populate_res_for_net(res);



        for (const auto& domain : res->search_domains) {

            ret = res_querydomainN(name, domain.c_str(), target, res, herrno);

res_cache.cpp

+58 −79
Original line number Diff line number Diff line
@@ -61,10 +61,14 @@ 
#include "DnsStats.h"

#include "res_debug.h"

#include "resolv_private.h"

#include "util.h"



using android::base::StringAppendF;

using android::net::DnsQueryEvent;

using android::net::DnsStats;

using android::net::PROTO_DOT;

using android::net::PROTO_TCP;

using android::net::PROTO_UDP;

using android::netdutils::DumpWriter;

using android::netdutils::IPSockAddr;
@@ -936,7 +940,7 @@ struct resolv_cache_info { 
    struct resolv_cache_info* next;

    int nscount;

    std::vector<std::string> nameservers;

    struct addrinfo* nsaddrinfo[MAXNS];  // TODO: Use struct sockaddr_storage.

    std::vector<IPSockAddr> nameserverSockAddrs;

    int revision_id;  // # times the nameservers have been replaced

    res_params params;

    struct res_stats nsstats[MAXNS];
@@ -1484,6 +1488,21 @@ std::vector<std::string> filter_nameservers(const std::vector<std::string>& serv 
    return res;

}



bool isValidServer(const std::string& server) {

    const addrinfo hints = {

            .ai_family = AF_UNSPEC,

            .ai_socktype = SOCK_DGRAM,

    };

    addrinfo* result = nullptr;

    if (int err = getaddrinfo_numeric(server.c_str(), "53", hints, &result); err != 0) {

        LOG(WARNING) << __func__ << ": getaddrinfo_numeric(" << server

                     << ") = " << gai_strerror(err);

        return false;

    }

    freeaddrinfo(result);

    return true;

}



}  // namespace



int resolv_set_nameservers(unsigned netid, const std::vector<std::string>& servers,
@@ -1495,24 +1514,11 @@ int resolv_set_nameservers(unsigned netid, const std::vector<std::string>& serve 


    // Parse the addresses before actually locking or changing any state, in case there is an error.

    // As a side effect this also reduces the time the lock is kept.

    // TODO: find a better way to replace addrinfo*, something like std::vector<SafeAddrinfo>

    addrinfo* nsaddrinfo[MAXNS];

    for (int i = 0; i < numservers; i++) {

        // The addrinfo structures allocated here are freed in free_nameservers_locked().

        const addrinfo hints = {

                .ai_flags = AI_NUMERICHOST,

                .ai_family = AF_UNSPEC,

                .ai_socktype = SOCK_DGRAM,

        };

        const int rt = getaddrinfo_numeric(nameservers[i].c_str(), "53", hints, &nsaddrinfo[i]);

        if (rt != 0) {

            for (int j = 0; j < i; j++) {

                freeaddrinfo(nsaddrinfo[j]);

            }

            LOG(INFO) << __func__ << ": getaddrinfo_numeric(" << nameservers[i]

                      << ") = " << gai_strerror(rt);

            return -EINVAL;

        }

    std::vector<IPSockAddr> ipSockAddrs;

    ipSockAddrs.reserve(nameservers.size());

    for (const auto& server : nameservers) {

        if (!isValidServer(server)) return -EINVAL;

        ipSockAddrs.push_back(IPSockAddr::toIPSockAddr(server, 53));

    }



    std::lock_guard guard(cache_mutex);
@@ -1528,10 +1534,10 @@ int resolv_set_nameservers(unsigned netid, const std::vector<std::string>& serve 
        free_nameservers_locked(cache_info);

        cache_info->nameservers = std::move(nameservers);

        for (int i = 0; i < numservers; i++) {

            cache_info->nsaddrinfo[i] = nsaddrinfo[i];

            LOG(INFO) << __func__ << ": netid = " << netid

                      << ", addr = " << cache_info->nameservers[i];

        }

        cache_info->nameserverSockAddrs = std::move(ipSockAddrs);

        cache_info->nscount = numservers;

    } else {

        if (cache_info->params.max_samples != old_max_samples) {
@@ -1542,23 +1548,15 @@ int resolv_set_nameservers(unsigned netid, const std::vector<std::string>& serve 
            // under which servers are considered usable.

            res_cache_clear_stats_locked(cache_info);

        }

        for (int j = 0; j < numservers; j++) {

            freeaddrinfo(nsaddrinfo[j]);

        }

    }



    // Always update the search paths. Cache-flushing however is not necessary,

    // since the stored cache entries do contain the domain, not just the host name.

    cache_info->search_domains = filter_domains(domains);



    std::vector<IPSockAddr> serverSockAddrs;

    serverSockAddrs.reserve(cache_info->nameservers.size());

    for (const auto& server : cache_info->nameservers) {

        serverSockAddrs.push_back(IPSockAddr::toIPSockAddr(server, 53));

    }



    if (!cache_info->dnsStats->setServers(serverSockAddrs, android::net::PROTO_TCP) ||

        !cache_info->dnsStats->setServers(serverSockAddrs, android::net::PROTO_UDP)) {

    // Setup stats for cleartext dns servers.

    if (!cache_info->dnsStats->setServers(cache_info->nameserverSockAddrs, PROTO_TCP) ||

        !cache_info->dnsStats->setServers(cache_info->nameserverSockAddrs, PROTO_UDP)) {

        LOG(WARNING) << __func__ << ": netid = " << netid << ", failed to set dns stats";

        return -EINVAL;

    }
@@ -1580,45 +1578,39 @@ static bool resolv_is_nameservers_equal(const std::vector<std::string>& oldServe 
}



static void free_nameservers_locked(resolv_cache_info* cache_info) {

    int i;

    for (i = 0; i < cache_info->nscount; i++) {

        cache_info->nameservers.clear();

        if (cache_info->nsaddrinfo[i] != nullptr) {

            freeaddrinfo(cache_info->nsaddrinfo[i]);

            cache_info->nsaddrinfo[i] = nullptr;

        }

    }

    cache_info->nscount = 0;

    cache_info->nameservers.clear();

    cache_info->nameserverSockAddrs.clear();

    res_cache_clear_stats_locked(cache_info);

}



void _resolv_populate_res_for_net(res_state statp) {

    if (statp == NULL) {

void resolv_populate_res_for_net(ResState* statp) {

    if (statp == nullptr) {

        return;

    }

    LOG(INFO) << __func__ << ": netid=" << statp->netid;



    std::lock_guard guard(cache_mutex);

    resolv_cache_info* info = find_cache_info_locked(statp->netid);

    if (info != NULL) {

        int nserv;

        struct addrinfo* ai;

        for (nserv = 0; nserv < MAXNS; nserv++) {

            ai = info->nsaddrinfo[nserv];

            if (ai == NULL) {

                break;

            }

    if (info == nullptr) return;



    // TODO: Convert nsaddrs[] to c++ container and remove the size-checking.

    const int serverNum = std::min(MAXNS, static_cast<int>(info->nameserverSockAddrs.size()));



    for (int nserv = 0; nserv < serverNum; nserv++) {

        sockaddr_storage ss = info->nameserverSockAddrs.at(nserv);



            if ((size_t)ai->ai_addrlen <= sizeof(statp->nsaddrs[0])) {

                memcpy(&statp->nsaddrs[nserv], ai->ai_addr, ai->ai_addrlen);

        if (auto sockaddr_len = sockaddrSize(ss); sockaddr_len != 0) {

            memcpy(&statp->nsaddrs[nserv], &ss, sockaddr_len);

        } else {

                LOG(INFO) << __func__ << ": found too long addrlen";

            LOG(WARNING) << __func__ << ": can't get sa_len from "

                         << info->nameserverSockAddrs.at(nserv);

        }

    }

        statp->nscount = nserv;



    statp->nscount = serverNum;

    statp->search_domains = info->search_domains;

}

}



/* Resolver reachability statistics. */
@@ -1666,31 +1658,18 @@ int android_net_res_stats_get_info_for_net(unsigned netid, int* nscount, 
            return -1;

        }

        int i;

        for (i = 0; i < info->nscount; i++) {

            // Verify that the following assumptions are held, failure indicates corruption:

            //  - getaddrinfo() may never return a sockaddr > sockaddr_storage

            //  - all addresses are valid

            //  - there is only one address per addrinfo thanks to numeric resolution

            int addrlen = info->nsaddrinfo[i]->ai_addrlen;

            if (addrlen < (int) sizeof(struct sockaddr) || addrlen > (int) sizeof(servers[0])) {

                LOG(INFO) << __func__ << ": nsaddrinfo[" << i << "].ai_addrlen == " << addrlen;

                errno = EMSGSIZE;

                return -1;

            }

            if (info->nsaddrinfo[i]->ai_addr == NULL) {

                LOG(INFO) << __func__ << ": nsaddrinfo[" << i << "].ai_addr == NULL";

                errno = ENOENT;

                return -1;

            }

            if (info->nsaddrinfo[i]->ai_next != NULL) {

                LOG(INFO) << __func__ << ": nsaddrinfo[" << i << "].ai_next != NULL";

                errno = ENOTUNIQ;

        *nscount = info->nscount;



        // It shouldn't happen, but just in case of buffer overflow.

        if (info->nscount != static_cast<int>(info->nameserverSockAddrs.size())) {

            LOG(INFO) << __func__ << ": nscount " << info->nscount

                      << " != " << info->nameserverSockAddrs.size();

            errno = EFAULT;

            return -1;

        }

        }

        *nscount = info->nscount;



        for (i = 0; i < info->nscount; i++) {

            memcpy(&servers[i], info->nsaddrinfo[i]->ai_addr, info->nsaddrinfo[i]->ai_addrlen);

            servers[i] = info->nameserverSockAddrs.at(i);

            stats[i] = info->nsstats[i];

        }

res_init.cpp

+1 −1
Original line number Diff line number Diff line
@@ -111,7 +111,7 @@ void res_init(ResState* statp, const struct android_net_context* _Nonnull netcon 
    }



    // The following dummy initialization is probably useless because

    // it's overwritten later by _resolv_populate_res_for_net().

    // it's overwritten later by resolv_populate_res_for_net().

    // TODO: check if it's safe to remove.

    const sockaddr_union u{

            .sin.sin_addr.s_addr = INADDR_ANY,

res_query.cpp

+1 −1
Original line number Diff line number Diff line
@@ -256,7 +256,7 @@ int res_nsearch(res_state statp, const char* name, /* domain name */ 
         * be loaded once for the thread instead of each

         * time a query is tried.

         */

        _resolv_populate_res_for_net(statp);

        resolv_populate_res_for_net(statp);



        for (const auto& domain : statp->search_domains) {

            if (domain == "." || domain == "") ++root_on_list;