From: Bert Hubert Date: Sun, 18 Mar 2007 19:34:08 +0000 (+0000) Subject: first round of tcpreceiver cleanups - should fix memory leaks, plus add proper suppor... X-Git-Tag: pdns-2.9.21~65 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ff76e8b4bdb49bc5c0d64bc8021a2d1812b8a1a8;p=pdns first round of tcpreceiver cleanups - should fix memory leaks, plus add proper support for recursive TCP questions, which are now handled correctly! git-svn-id: svn://svn.powerdns.com/pdns/trunk/pdns@980 d19b8d6e-7fed-0310-83ef-9ca221ded41b --- diff --git a/pdns/packethandler.cc b/pdns/packethandler.cc index d111be8f6..1754f1ef2 100644 --- a/pdns/packethandler.cc +++ b/pdns/packethandler.cc @@ -553,9 +553,20 @@ bool validDNSName(const string &name) return true; } -//! Called by the Distributor to ask a question. Returns 0 in case of an error DNSPacket *PacketHandler::question(DNSPacket *p) { + bool shouldRecurse=false; + DNSPacket *ret=questionOrRecurse(p, &shouldRecurse); + if(shouldRecurse) { + DP->sendPacket(p); + } + return ret; +} + +//! Called by the Distributor to ask a question. Returns 0 in case of an error +DNSPacket *PacketHandler::questionOrRecurse(DNSPacket *p, bool *shouldRecurse) +{ + *shouldRecurse=false; DNSResourceRecord rr; SOAData sd; sd.db=0; @@ -738,7 +749,6 @@ DNSPacket *PacketHandler::question(DNSPacket *p) // RECURSION CUT-OUT! - bool weAuth; int zoneId; zoneId=-1; @@ -750,10 +760,12 @@ DNSPacket *PacketHandler::question(DNSPacket *p) if(p->d.rd && d_doRecursion && !weAuth) { - if(DP->sendPacket(p)) { + if(DP->recurseFor(p)) { + *shouldRecurse=true; delete r; return 0; } + else noCache=true; } diff --git a/pdns/packethandler.hh b/pdns/packethandler.hh index 34f233a30..3fcd1569b 100644 --- a/pdns/packethandler.hh +++ b/pdns/packethandler.hh @@ -69,7 +69,7 @@ public: T **d_guard; }; - + DNSPacket *questionOrRecurse(DNSPacket *, bool* shouldRecurse); //!< hand us a DNS packet with a question, we'll tell you answer, or that you should recurse DNSPacket *question(DNSPacket *); //!< hand us a DNS packet with a question, we give you an answer PacketHandler(); ~PacketHandler(); // defined in packethandler.cc, and does --count diff --git a/pdns/tcpreceiver.cc b/pdns/tcpreceiver.cc index 00b723aa0..f7232e468 100644 --- a/pdns/tcpreceiver.cc +++ b/pdns/tcpreceiver.cc @@ -1,6 +1,6 @@ /* PowerDNS Versatile Database Driven Nameserver - Copyright (C) 2002-2006 PowerDNS.COM BV + Copyright (C) 2002-2007 PowerDNS.COM BV This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License version 2 @@ -39,6 +39,7 @@ #include "statbag.hh" #include "resolver.hh" #include "communicator.hh" +using namespace boost; extern PacketCache PC; extern StatBag S; @@ -54,11 +55,11 @@ PacketHandler *TCPNameserver::s_P; int TCPNameserver::s_timeout; NetmaskGroup TCPNameserver::d_ng; -int TCPNameserver::sendDelPacket(DNSPacket *p, int outsock) + +int TCPNameserver::sendPacket(shared_ptr p, int outsock) { const char *buf=p->getData(); int res=sendData(buf, p->len, outsock); - delete p; return res; } @@ -82,7 +83,6 @@ void *TCPNameserver::launcher(void *data) return 0; } - int TCPNameserver::readLength(int fd, ComboAddress *remote) { int bytesLeft=2; @@ -126,9 +126,61 @@ void TCPNameserver::getQuestion(int fd, char *mesg, int pktlen, const ComboAddre throw AhuException("Remote TCP client "+remote.toString()+" closed connection"); } +static void proxyQuestion(shared_ptr packet) +{ + int sock=socket(AF_INET, SOCK_STREAM, 0); + if(sock < 0) + throw AhuException("Error making TCP connection socket to recursor: "+stringerror()); + + try { + ServiceTuple st; + st.port=53; + parseService(arg()["recursor"],st); + + ComboAddress recursor(st.host, st.port); + if(connect(sock, (struct sockaddr*)&recursor, recursor.getSocklen()) < 0) { + throw AhuException("Error making TCP connection to recursor "+st.host+": "+stringerror()); + } + const string &buffer=packet->getString(); + + uint16_t len=htons(buffer.length()), slen; + + if(write(sock, &len, 2) != 2 || write(sock, buffer.c_str(), buffer.length()) != buffer.length()) + throw AhuException("Error sending data to recursor"); + + int ret; + + ret=read(sock, &len, 2); + if(ret!=2) { + throw AhuException("Error reading data from recursor"); + } + len=ntohs(len); + + char answer[len]; + ret=read(sock, answer, len); + if(ret!=len) + throw AhuException("Error reading data from recursor"); + + slen=htons(len); + ret=write(packet->getSocket(), &slen, 2); + if(ret != 2) + throw AhuException("Error reading data from recursor"); + + ret=write(packet->getSocket(), answer, len); + if(ret != len) + throw AhuException("Error reading data from recursor"); + } + catch(...) { + close(sock); + throw; + } + close(sock); + return; +} + void *TCPNameserver::doConnection(void *data) { - DNSPacket *packet = NULL; + shared_ptr packet; // Fix gcc-4.0 error (on AMD64) int fd=(int)(long)data; // gotta love C (generates a harmless warning on opteron) pthread_detach(pthread_self()); @@ -153,13 +205,10 @@ void *TCPNameserver::doConnection(void *data) getQuestion(fd,mesg,pktlen,remote); S.inc("tcp-queries"); - if (packet != NULL) - delete packet; - - packet=new DNSPacket; - + packet=shared_ptr(new DNSPacket); packet->setRemote(&remote); packet->d_tcp=true; + packet->setSocket(fd); if(packet->parse(mesg, pktlen)<0) break; @@ -169,68 +218,43 @@ void *TCPNameserver::doConnection(void *data) continue; } + shared_ptr reply; - if(packet->d.rd && arg().mustDo("recursor")) { - // now what - // this is a pretty rare event all in all, so we can afford to be slow - - // this code SHOULD attempt to answer from the local cache first! - S.inc("recursing-questions"); - Resolver res; - unsigned int len; - DLOG(L<<"About to hand query to recursor"<getRaw(),packet->len,&len); - DLOG(L<<"got an answer from recursor: "<d.rd && (PC.get(packet, cached))) { // short circuit - does the PacketCache recognize this question? + shared_ptr cached= shared_ptr(new DNSPacket); + + if(!packet->d.rd && (PC.get(packet.get(), cached.get()))) { // short circuit - does the PacketCache recognize this question? cached->setRemote(&packet->remote); cached->spoofID(packet->d.id); - if(sendDelPacket(cached, fd)<0) + if(sendPacket(cached, fd)<0) goto out; - + S.inc("tcp-answers"); continue; } - else - delete cached; - - DNSPacket *reply; + { Lock l(&s_plock); if(!s_P) { L<question(packet); // we really need to ask the backend :-) + bool shouldRecurse; + reply=shared_ptr(s_P->questionOrRecurse(packet.get(), &shouldRecurse)); // we really need to ask the backend :-) + if(shouldRecurse) { + proxyQuestion(packet); + continue; + } } - delete packet; - packet = NULL; - if(!reply) // unable to write an answer? break; S.inc("tcp-answers"); - sendDelPacket(reply, fd); + sendPacket(reply, fd); } - out: - if (packet != NULL) - delete packet; + ; } catch(DBException &e) { Lock l(&s_plock); @@ -258,7 +282,7 @@ void *TCPNameserver::doConnection(void *data) return 0; } -bool TCPNameserver::canDoAXFR(DNSPacket *q) +bool TCPNameserver::canDoAXFR(shared_ptr q) { if(arg().mustDo("disable-axfr")) return false; @@ -277,20 +301,20 @@ bool TCPNameserver::canDoAXFR(DNSPacket *q) } /** do the actual zone transfer. Return 0 in case of error, 1 in case of success */ -int TCPNameserver::doAXFR(const string &target, DNSPacket *q, int outsock) +int TCPNameserver::doAXFR(const string &target, shared_ptr q, int outsock) { - DNSPacket *outpacket=0; + shared_ptr outpacket; if(!canDoAXFR(q)) { L<getRemote()<replyPacket(); + outpacket=shared_ptr(q->replyPacket()); outpacket->setRcode(RCode::Refused); // FIXME: should actually figure out if we are auth over a zone, and send out 9 if we aren't - sendDelPacket(outpacket,outsock); + sendPacket(outpacket,outsock); return 0; } L<getRemote()<replyPacket(); + outpacket=shared_ptr(q->replyPacket()); DNSResourceRecord soa; DNSResourceRecord rr; @@ -311,7 +335,7 @@ int TCPNameserver::doAXFR(const string &target, DNSPacket *q, int outsock) if(!s_P->getBackend()->getSOA(target,sd)) { L<setRcode(9); // 'NOTAUTH' - sendDelPacket(outpacket,outsock); + sendPacket(outpacket,outsock); return 0; } @@ -322,7 +346,7 @@ int TCPNameserver::doAXFR(const string &target, DNSPacket *q, int outsock) if(!P.getBackend()->getSOA(target, sd)) { L<setRcode(9); // 'NOTAUTH' - sendDelPacket(outpacket,outsock); + sendPacket(outpacket,outsock); return 0; } @@ -336,7 +360,7 @@ int TCPNameserver::doAXFR(const string &target, DNSPacket *q, int outsock) if(!sd.db || sd.db==(DNSBackend *)-1) { L<setRcode(RCode::ServFail); - sendDelPacket(outpacket,outsock); + sendPacket(outpacket,outsock); return 0; } @@ -348,14 +372,14 @@ int TCPNameserver::doAXFR(const string &target, DNSPacket *q, int outsock) if(!(B->list(target, sd.domain_id))) { L<setRcode(2); // 'SERVFAIL' - sendDelPacket(outpacket,outsock); + sendPacket(outpacket,outsock); return 0; } /* write first part of answer */ DLOG(L<<"Sending out SOA"<addRecord(soa); // AXFR format begins and ends with a SOA record, so we add one - sendDelPacket(outpacket, outsock); + sendPacket(outpacket, outsock); /* now write all other records */ @@ -364,7 +388,7 @@ int TCPNameserver::doAXFR(const string &target, DNSPacket *q, int outsock) if(arg().mustDo("strict-rfc-axfrs")) chunk=1; - outpacket=q->replyPacket(); + outpacket=shared_ptr(q->replyPacket()); outpacket->setCompress(false); while(B->get(rr)) { @@ -376,23 +400,23 @@ int TCPNameserver::doAXFR(const string &target, DNSPacket *q, int outsock) if(!((++count)%chunk)) { count=0; - if(sendDelPacket(outpacket, outsock) < 0) // FIXME: this leaks memory! + if(sendPacket(outpacket, outsock) < 0) return 0; - outpacket=q->replyPacket(); + outpacket=shared_ptr(q->replyPacket()); outpacket->setCompress(false); // FIXME: Subsequent messages SHOULD NOT have a question section, though the final message MAY. } } if(count) { - sendDelPacket(outpacket, outsock); + sendPacket(outpacket, outsock); } DLOG(L<<"Done writing out records"<replyPacket(); + outpacket=shared_ptr(q->replyPacket()); outpacket->addRecord(soa); - sendDelPacket(outpacket, outsock); + sendPacket(outpacket, outsock); DLOG(L<<"last packet - close"<getRemote()<<" finished"<locals6; stringtok(locals6,arg()["local-ipv6"]," ,"); - if(locals.empty() && locals6.empty()) throw AhuException("No local address specified"); @@ -434,30 +457,12 @@ TCPNameserver::TCPNameserver() FD_ZERO(&d_rfds); for(vector::const_iterator laddr=locals.begin();laddr!=locals.end();++laddr) { - struct sockaddr_in local; int s=socket(AF_INET,SOCK_STREAM,0); if(s<0) throw AhuException("Unable to acquire TCP socket: "+stringerror()); - - memset(&local,0,sizeof(local)); - local.sin_family=AF_INET; - - struct hostent *h; - - if ( *laddr == "0.0.0.0" ) - { - local.sin_addr.s_addr = INADDR_ANY; - } - else - { - h=gethostbyname(laddr->c_str()); - - if(!h) - throw AhuException("Unable to resolve local address '"+*laddr+"'"); - local.sin_addr.s_addr=*(int*)h->h_addr; - } + ComboAddress local(*laddr, arg().asNum("local-port")); int tmp=1; if(setsockopt(s,SOL_SOCKET,SO_REUSEADDR,(char*)&tmp,sizeof tmp)<0) { @@ -465,21 +470,18 @@ TCPNameserver::TCPNameserver() exit(1); } - local.sin_port=htons(arg().asNum("local-port")); - - if(bind(s, (sockaddr*)&local,sizeof(local))<0) { + if(bind(s, (sockaddr*)&local, local.getSocklen())<0) { L<::const_iterator laddr=locals6.begin();laddr!=locals6.end();++laddr) { int s=socket(AF_INET6,SOCK_STREAM,0); @@ -487,23 +489,7 @@ TCPNameserver::TCPNameserver() if(s<0) throw AhuException("Unable to acquire TCPv6 socket: "+stringerror()); - sockaddr_in6 locala; - memset(&locala, 0, sizeof(locala)); - locala.sin6_port=htons(arg().asNum("local-port")); - locala.sin6_family=AF_INET6; - - if(!inet_pton(AF_INET6, laddr->c_str(), (void *)&locala.sin6_addr)) { - addrinfo *addrinfos; - addrinfo hints; - memset(&hints,0,sizeof(hints)); - hints.ai_socktype=SOCK_STREAM; - hints.ai_family=AF_INET6; - - if(getaddrinfo(laddr->c_str(),arg()["local-port"].c_str(),&hints,&addrinfos)) - throw AhuException("Unable to resolve local IPv6 address '"+*laddr+"'"); - - memcpy(&locala,addrinfos->ai_addr,addrinfos->ai_addrlen); - } + ComboAddress local(*laddr, arg().asNum("local-port")); int tmp=1; if(setsockopt(s,SOL_SOCKET,SO_REUSEADDR,(char*)&tmp,sizeof tmp)<0) { @@ -511,14 +497,13 @@ TCPNameserver::TCPNameserver() exit(1); } - - if(bind(s, (const sockaddr*)&locala, sizeof(locala))<0) { + if(bind(s, (const sockaddr*)&local, local.getSocklen())<0) { L< +#include #ifndef WIN32 # include @@ -47,11 +48,11 @@ public: void go(); private: - static int sendDelPacket(DNSPacket *p, int outsock); + static int sendPacket(boost::shared_ptr p, int outsock); static int readLength(int fd, ComboAddress *remote); static void getQuestion(int fd, char *mesg, int pktlen, const ComboAddress& remote); - static int doAXFR(const string &target, DNSPacket *q, int outsock); - static bool canDoAXFR(DNSPacket *q); + static int doAXFR(const string &target, boost::shared_ptr q, int outsock); + static bool canDoAXFR(boost::shared_ptr q); static void *doConnection(void *data); static void *launcher(void *data); void thread(void);