Make resolv_gethostbyname() re-entrant
The new call signature is closer to gethostbyname2_r() and no longer
relies on thread-local storage for its buffers. res_get_static() is now
down to a single caller.
Test: m, flash, atest
Change-Id: Ic6e2c0292fece31c2586a81d8fb6a8df20c4f9f5
diff --git a/DnsProxyListener.cpp b/DnsProxyListener.cpp
index 974cd51..e1a3b89 100644
--- a/DnsProxyListener.cpp
+++ b/DnsProxyListener.cpp
@@ -75,6 +75,7 @@
constexpr int MAX_QUERIES_PER_UID = 256;
// Max packet size for answer, sync with getaddrinfo.c
+// TODO: switch to dynamically allocated buffers with std::vector
constexpr int MAXPACKET = 8 * 1024;
android::netdutils::OperationLimiter<uid_t> queryLimiter(MAX_QUERIES_PER_UID);
@@ -1033,7 +1034,8 @@
free(mName);
}
-void DnsProxyListener::GetHostByNameHandler::doDns64Synthesis(int32_t* rv, struct hostent** hpp,
+void DnsProxyListener::GetHostByNameHandler::doDns64Synthesis(int32_t* rv, hostent* hbuf, char* buf,
+ size_t buflen, struct hostent** hpp,
NetworkDnsEventReported* event) {
// Don't have to consider family AF_UNSPEC case because gethostbyname{, 2} only supports
// family AF_INET or AF_INET6.
@@ -1051,7 +1053,7 @@
// If caller wants IPv6 answers but no data, try to query IPv4 answers for synthesis
const uid_t uid = mClient->getUid();
if (queryLimiter.start(uid)) {
- *rv = android_gethostbynamefornetcontext(mName, AF_INET, &mNetContext, hpp, event);
+ *rv = resolv_gethostbyname(mName, AF_INET, hbuf, buf, buflen, &mNetContext, hpp, event);
queryLimiter.finish(uid);
if (*rv) {
*rv = EAI_NODATA; // return original error code
@@ -1074,11 +1076,14 @@
maybeFixupNetContext(&mNetContext);
const uid_t uid = mClient->getUid();
hostent* hp = nullptr;
+ hostent hbuf;
+ char tmpbuf[MAXPACKET];
int32_t rv = 0;
NetworkDnsEventReported event;
initDnsEvent(&event);
if (queryLimiter.start(uid)) {
- rv = android_gethostbynamefornetcontext(mName, mAf, &mNetContext, &hp, &event);
+ rv = resolv_gethostbyname(mName, mAf, &hbuf, tmpbuf, sizeof tmpbuf, &mNetContext, &hp,
+ &event);
queryLimiter.finish(uid);
} else {
rv = EAI_MEMORY;
@@ -1086,12 +1091,12 @@
<< ", max concurrent queries reached";
}
- doDns64Synthesis(&rv, &hp, &event);
+ doDns64Synthesis(&rv, &hbuf, tmpbuf, sizeof tmpbuf, &hp, &event);
const int32_t latencyUs = saturate_cast<int32_t>(s.timeTakenUs());
event.set_latency_micros(latencyUs);
event.set_event_type(EVENT_GETHOSTBYNAME);
- LOG(DEBUG) << "GetHostByNameHandler::run: errno: " << (hp ? "success" : strerror(errno));
+ LOG(DEBUG) << "GetHostByNameHandler::run: result: " << gai_strerror(rv);
bool success = true;
if (hp) {
diff --git a/DnsProxyListener.h b/DnsProxyListener.h
index 9090ac2..7af36c5 100644
--- a/DnsProxyListener.h
+++ b/DnsProxyListener.h
@@ -84,7 +84,8 @@
std::string threadName();
private:
- void doDns64Synthesis(int32_t* rv, hostent** hpp, NetworkDnsEventReported* event);
+ void doDns64Synthesis(int32_t* rv, hostent* hbuf, char* buf, size_t buflen, hostent** hpp,
+ NetworkDnsEventReported* event);
SocketClient* mClient; // ref counted
char* mName; // owned. TODO: convert to std::string.
diff --git a/gethnamaddr.cpp b/gethnamaddr.cpp
index a2e88b4..13f3b19 100644
--- a/gethnamaddr.cpp
+++ b/gethnamaddr.cpp
@@ -117,12 +117,6 @@
NetworkDnsEventReported* event);
static int dns_gethtbyname(const char* name, int af, getnamaddr* info);
-static int gethostbyname_internal(const char* name, int af, res_state res, hostent* hp, char* hbuf,
- size_t hbuflen, const android_net_context* netcontext,
- NetworkDnsEventReported* event);
-static int gethostbyname_internal_real(const char* name, int af, hostent* hp, char* buf,
- size_t buflen);
-
static int android_gethostbyaddrfornetcontext_proxy_internal(const void*, socklen_t, int,
struct hostent*, char*, size_t,
const struct android_net_context*,
@@ -401,13 +395,16 @@
return NULL;
}
-static int gethostbyname_internal_real(const char* name, int af, hostent* hp, char* buf,
- size_t buflen) {
+int resolv_gethostbyname(const char* name, int af, hostent* hp, char* buf, size_t buflen,
+ const android_net_context* netcontext, hostent** result,
+ NetworkDnsEventReported* event) {
getnamaddr info;
+
+ res_state res = res_get_state();
+ if (res == nullptr) return EAI_MEMORY;
+ res_setnetcontext(res, netcontext, event);
+
size_t size;
-
- _DIAGASSERT(name != NULL);
-
switch (af) {
case AF_INET:
size = NS_INADDRSZ;
@@ -463,6 +460,7 @@
int error = dns_gethtbyname(name, af, &info);
if (error != 0) return error;
}
+ *result = hp;
return 0;
nospc:
return EAI_MEMORY;
@@ -481,17 +479,10 @@
buf += size;
buflen -= size;
HENT_SCOPY(hp->h_name, name, buf, buflen);
+ *result = hp;
return 0;
}
-// very similar in proxy-ness to android_getaddrinfo_proxy
-static int gethostbyname_internal(const char* name, int af, res_state res, hostent* hp, char* hbuf,
- size_t hbuflen, const android_net_context* netcontext,
- NetworkDnsEventReported* event) {
- res_setnetcontext(res, netcontext, event);
- return gethostbyname_internal_real(name, af, hp, hbuf, hbuflen);
-}
-
static int android_gethostbyaddrfornetcontext_real(const void* addr, socklen_t len, int af,
struct hostent* hp, char* buf, size_t buflen,
const struct android_net_context* netcontext,
@@ -554,7 +545,6 @@
// the error code of the caller does not currently return to netd.
struct hostent* netbsd_gethostent_r(FILE* hf, struct hostent* hent, char* buf, size_t buflen,
int* he) {
- const size_t line_buf_size = sizeof(res_get_static()->hostbuf);
char *name;
char* cp;
int af, len;
@@ -569,11 +559,8 @@
}
char* p = NULL;
- /* Allocate a new space to read file lines like upstream does.
- * To keep reentrancy we cannot use res_get_static()->hostbuf here,
- * as the buffer may be used to store content for a previous hostent
- * returned by non-reentrant functions like gethostbyname().
- */
+ // Allocate a new space to read file lines like upstream does.
+ const size_t line_buf_size = MAXPACKET;
if ((p = (char*) malloc(line_buf_size)) == NULL) {
goto nospc;
}
@@ -800,27 +787,6 @@
return EAI_MEMORY;
}
-/*
- * Non-reentrant versions.
- */
-
-int android_gethostbynamefornetcontext(const char* name, int af,
- const struct android_net_context* netcontext, hostent** hp,
- NetworkDnsEventReported* event) {
- assert(event != nullptr);
-
- res_state res = res_get_state();
- if (res == NULL) return EAI_MEMORY;
- res_static* rs = res_get_static(); // For thread-safety.
- int error;
- error = gethostbyname_internal(name, af, res, &rs->host, rs->hostbuf, sizeof(rs->hostbuf),
- netcontext, event);
- if (error == 0) {
- *hp = &rs->host;
- }
- return error;
-}
-
int android_gethostbyaddrfornetcontext(const void* addr, socklen_t len, int af,
const struct android_net_context* netcontext, hostent** hp,
NetworkDnsEventReported* event) {
diff --git a/gethnamaddr.h b/gethnamaddr.h
index bfdb14e..d328536 100644
--- a/gethnamaddr.h
+++ b/gethnamaddr.h
@@ -20,9 +20,17 @@
#include "netd_resolv/resolv.h" // struct android_net_context
#include "stats.pb.h"
+/*
+ * Error code extending EAI_* codes defined in bionic/libc/include/netdb.h.
+ * This error code, including EAI_*, returned from android_getaddrinfofornetcontext()
+ * and resolv_gethostbyname() are used for DNS metrics.
+ */
+#define NETD_RESOLV_TIMEOUT 255 // consistent with RCODE_TIMEOUT
+
// This is the entry point for the gethostbyname() family of legacy calls.
-int android_gethostbynamefornetcontext(const char*, int, const android_net_context*, hostent**,
- android::net::NetworkDnsEventReported*);
+int resolv_gethostbyname(const char* name, int af, hostent* hp, char* buf, size_t buflen,
+ const android_net_context* netcontext, hostent** result,
+ android::net::NetworkDnsEventReported*);
// This is the entry point for the gethostbyaddr() family of legacy calls.
int android_gethostbyaddrfornetcontext(const void*, socklen_t, int, const android_net_context*,
diff --git a/include/netd_resolv/resolv.h b/include/netd_resolv/resolv.h
index 17cf2aa..afd63f5 100644
--- a/include/netd_resolv/resolv.h
+++ b/include/netd_resolv/resolv.h
@@ -44,13 +44,6 @@
#define MARK_UNSET 0u
/*
- * Error code extending EAI_* codes defined in bionic/libc/include/netdb.h.
- * This error code, including EAI_*, returned from android_getaddrinfofornetcontext()
- * and android_gethostbynamefornetcontext() are used for DNS metrics.
- */
-#define NETD_RESOLV_TIMEOUT 255 // consistent with RCODE_TIMEOUT
-
-/*
* A struct to capture context relevant to network operations.
*
* Application and DNS netids/marks can differ from one another under certain
diff --git a/resolv_unit_test.cpp b/resolv_unit_test.cpp
index f9accd0..691b978 100644
--- a/resolv_unit_test.cpp
+++ b/resolv_unit_test.cpp
@@ -39,6 +39,9 @@
using android::net::NetworkDnsEventReported;
using android::netdutils::ScopedAddrinfo;
+// The buffer size of resolv_gethostbyname().
+constexpr unsigned int MAXPACKET = 8 * 1024;
+
class TestBase : public ::testing::Test {
protected:
struct DnsMessage {
@@ -707,9 +710,11 @@
dns.clearQueries();
hostent* hp = nullptr;
+ hostent hbuf;
+ char tmpbuf[MAXPACKET];
NetworkDnsEventReported event;
- int rv = android_gethostbynamefornetcontext("jiababuei", config.ai_family, &mNetcontext,
- &hp, &event);
+ int rv = resolv_gethostbyname("jiababuei", config.ai_family, &hbuf, tmpbuf, sizeof(tmpbuf),
+ &mNetcontext, &hp, &event);
EXPECT_EQ(0, rv);
EXPECT_TRUE(hp != nullptr);
EXPECT_EQ(1U, GetNumQueries(dns, host_name));
@@ -747,9 +752,11 @@
SCOPED_TRACE(StringPrintf("family: %d, config.name: %s", family, hostname));
struct hostent* hp = nullptr;
+ hostent hbuf;
+ char tmpbuf[MAXPACKET];
NetworkDnsEventReported event;
- int rv =
- android_gethostbynamefornetcontext(hostname, family, &mNetcontext, &hp, &event);
+ int rv = resolv_gethostbyname(hostname, family, &hbuf, tmpbuf, sizeof(tmpbuf),
+ &mNetcontext, &hp, &event);
EXPECT_EQ(nullptr, hp);
EXPECT_EQ(EAI_FAIL, rv);
}
@@ -767,8 +774,11 @@
// Want AAAA answer but DNS server has A answer only.
hostent* hp = nullptr;
+ hostent hbuf;
+ char tmpbuf[MAXPACKET];
NetworkDnsEventReported event;
- int rv = android_gethostbynamefornetcontext("v4only", AF_INET6, &mNetcontext, &hp, &event);
+ int rv = resolv_gethostbyname("v4only", AF_INET6, &hbuf, tmpbuf, sizeof tmpbuf, &mNetcontext,
+ &hp, &event);
EXPECT_LE(1U, GetNumQueries(dns, v4_host_name));
EXPECT_EQ(nullptr, hp);
EXPECT_EQ(EAI_NODATA, rv);
@@ -805,8 +815,11 @@
ASSERT_EQ(0, SetResolvers());
hostent* hp = nullptr;
+ hostent hbuf;
+ char tmpbuf[MAXPACKET];
NetworkDnsEventReported event;
- int rv = android_gethostbynamefornetcontext(host_name, AF_INET, &mNetcontext, &hp, &event);
+ int rv = resolv_gethostbyname(host_name, AF_INET, &hbuf, tmpbuf, sizeof tmpbuf,
+ &mNetcontext, &hp, &event);
EXPECT_EQ(nullptr, hp);
EXPECT_EQ(config.expected_eai_error, rv);
}
@@ -822,8 +835,11 @@
ASSERT_EQ(0, SetResolvers());
hostent* hp = nullptr;
+ hostent hbuf;
+ char tmpbuf[MAXPACKET];
NetworkDnsEventReported event;
- int rv = android_gethostbynamefornetcontext(host_name, AF_INET, &mNetcontext, &hp, &event);
+ int rv = resolv_gethostbyname(host_name, AF_INET, &hbuf, tmpbuf, sizeof tmpbuf, &mNetcontext,
+ &hp, &event);
EXPECT_EQ(NETD_RESOLV_TIMEOUT, rv);
}
@@ -852,9 +868,11 @@
StringPrintf("config.family: %d, config.name: %s", config.family, config.name));
struct hostent* hp = nullptr;
+ hostent hbuf;
+ char tmpbuf[MAXPACKET];
NetworkDnsEventReported event;
- int rv = android_gethostbynamefornetcontext(config.name, config.family, &mNetcontext, &hp,
- &event);
+ int rv = resolv_gethostbyname(config.name, config.family, &hbuf, tmpbuf, sizeof tmpbuf,
+ &mNetcontext, &hp, &event);
EXPECT_EQ(nullptr, hp);
EXPECT_EQ(EAI_FAIL, rv);
}
@@ -902,9 +920,11 @@
StringPrintf("family: %d, testHostName: %s", family, testHostName.c_str()));
struct hostent* hp = nullptr;
+ hostent hbuf;
+ char tmpbuf[MAXPACKET];
NetworkDnsEventReported event;
- int rv = android_gethostbynamefornetcontext(config.name, family, &mNetcontext, &hp,
- &event);
+ int rv = resolv_gethostbyname(config.name, family, &hbuf, tmpbuf, sizeof tmpbuf,
+ &mNetcontext, &hp, &event);
EXPECT_EQ(nullptr, hp);
EXPECT_EQ(EAI_FAIL, rv);
}
@@ -922,8 +942,11 @@
SCOPED_TRACE(StringPrintf("family: %d", family));
struct hostent* hp = nullptr;
+ hostent hbuf;
+ char tmpbuf[MAXPACKET];
NetworkDnsEventReported event;
- int rv = android_gethostbynamefornetcontext("hello", family, &mNetcontext, &hp, &event);
+ int rv = resolv_gethostbyname("hello", family, &hbuf, tmpbuf, sizeof tmpbuf, &mNetcontext,
+ &hp, &event);
EXPECT_EQ(nullptr, hp);
EXPECT_EQ(EAI_FAIL, rv);
}
@@ -938,7 +961,7 @@
// - Invalid length CNAME, or QNAME.
// - Unexpected amount of questions.
// - CNAME RDATA with the domain name which has null label(s).
-// TODO: Add test for android_gethostbynamefornetcontext().
+// TODO: Add test for resolv_gethostbyname().
// - Invalid parameters.
// - DNS response message parsing.
// - Unexpected type of resource record (RR).