Permalink
Browse files

We did not transmit the right 'local' socket address to Lua for TCP/I…

…P queries in the recursor. In addition, we

would attempt to lookup a filedescriptor that wasn't there in an unlocked map which could conceivably lead to crashes. Closes #1828, thanks Winfried for reporting
  • Loading branch information...
1 parent de04008 commit 50274292b811d42e757ebba13c822be9028bde8c @ahupowerdns ahupowerdns committed with Habbie Oct 16, 2014
Showing with 23 additions and 9 deletions.
  1. +22 −8 pdns/pdns_recursor.cc
  2. +1 −1 pdns/powerdns-example-script.lua
View
@@ -119,7 +119,8 @@ tcpListenSockets_t g_tcpListenSockets; // shared across threads, but this is f
int g_tcpTimeout;
unsigned int g_maxMThreads;
struct timeval g_now; // timestamp, updated (too) frequently
-map<int, ComboAddress> g_listenSocketsAddresses; // is shared across all threads right now
+typedef map<int, ComboAddress> listenSocketsAddresses_t; // is shared across all threads right now
+listenSocketsAddresses_t g_listenSocketsAddresses; // is shared across all threads right now
__thread MT_t* MT; // the big MTasker
@@ -507,7 +508,8 @@ void startDoResolve(void *p)
if(getEDNSOpts(dc->d_mdp, &edo) && !dc->d_tcp) {
maxanswersize = min(edo.d_packetsize, g_udpTruncationThreshold);
}
-
+ ComboAddress local;
+ listenSocketsAddresses_t::const_iterator lociter;
vector<DNSResourceRecord> ret;
vector<uint8_t> packet;
@@ -548,8 +550,19 @@ void startDoResolve(void *p)
if(!dc->d_mdp.d_header.rd)
sr.setCacheOnly();
- // if there is a RecursorLua active, and it 'took' the query in preResolve, we don't launch beginResolve
- if(!t_pdl->get() || !(*t_pdl)->preresolve(dc->d_remote, g_listenSocketsAddresses[dc->d_socket], dc->d_mdp.d_qname, QType(dc->d_mdp.d_qtype), ret, res, &variableAnswer)) {
+ local.sin4.sin_family = dc->d_remote.sin4.sin_family;
+
+ lociter = g_listenSocketsAddresses.find(dc->d_socket);
+ if(lociter != g_listenSocketsAddresses.end()) {
+ local = lociter->second;
+ }
+ else {
+ socklen_t len = local.getSocklen();
+ getsockname(dc->d_socket, (sockaddr*)&local, &len); // if this fails, we're ok with it
+ }
+
+ // if there is a RecursorLua active, and it 'took' the query in preResolve, we don't launch beginResolve
+ if(!t_pdl->get() || !(*t_pdl)->preresolve(dc->d_remote, local, dc->d_mdp.d_qname, QType(dc->d_mdp.d_qtype), ret, res, &variableAnswer)) {
res = sr.beginResolve(dc->d_mdp.d_qname, QType(dc->d_mdp.d_qtype), dc->d_mdp.d_qclass, ret);
if(t_pdl->get()) {
@@ -559,12 +572,12 @@ void startDoResolve(void *p)
if(i->qtype.getCode() == dc->d_mdp.d_qtype && i->d_place == DNSResourceRecord::ANSWER)
break;
if(i == ret.end())
- (*t_pdl)->nodata(dc->d_remote, g_listenSocketsAddresses[dc->d_socket], dc->d_mdp.d_qname, QType(dc->d_mdp.d_qtype), ret, res, &variableAnswer);
+ (*t_pdl)->nodata(dc->d_remote,local, dc->d_mdp.d_qname, QType(dc->d_mdp.d_qtype), ret, res, &variableAnswer);
}
else if(res == RCode::NXDomain)
- (*t_pdl)->nxdomain(dc->d_remote, g_listenSocketsAddresses[dc->d_socket], dc->d_mdp.d_qname, QType(dc->d_mdp.d_qtype), ret, res, &variableAnswer);
+ (*t_pdl)->nxdomain(dc->d_remote,local, dc->d_mdp.d_qname, QType(dc->d_mdp.d_qtype), ret, res, &variableAnswer);
- (*t_pdl)->postresolve(dc->d_remote, g_listenSocketsAddresses[dc->d_socket], dc->d_mdp.d_qname, QType(dc->d_mdp.d_qtype), ret, res, &variableAnswer);
+ (*t_pdl)->postresolve(dc->d_remote,local, dc->d_mdp.d_qname, QType(dc->d_mdp.d_qtype), ret, res, &variableAnswer);
}
}
@@ -1022,7 +1035,8 @@ void makeTCPServerSockets()
listen(fd, 128);
deferredAdd.push_back(make_pair(fd, handleNewTCPQuestion));
g_tcpListenSockets.push_back(fd);
-
+ // we don't need to update g_listenSocketsAddresses since it doesn't work for TCP/IP:
+ // - fd is not that which we know here, but returned from accept()
if(sin.sin4.sin_family == AF_INET)
L<<Logger::Error<<"Listening for TCP queries on "<< sin.toString() <<":"<<st.port<<endl;
else
@@ -5,7 +5,7 @@ end
function preresolve ( remoteip, domain, qtype )
- print ("prequery handler called for: ", remoteip, getlocaladdress(), domain, qtype)
+ print ("prequery handler called for: ", remoteip, "local: ", getlocaladdress(), domain, qtype)
pdnslog("a test message.. received query from "..remoteip.." on "..getlocaladdress(), pdns.loglevels.Info);
if endswith(domain, "f.f.7.7.b.1.2.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.e.f.ip6.arpa.")

0 comments on commit 5027429

Please sign in to comment.