Skip to content

Commit 5a38a56

Browse files
rgacognergacogne
committed
Handle exceptions raised by closesocket()
This was not very well handled, and could cause the PowerDNS process to terminate. This is especially nasty when `closesocket()` is called from a destructor, as we could already be dealing with an exception. (cherry picked from commit a7b68ae)
1 parent a2bee67 commit 5a38a56

File tree

7 files changed

+124
-24
lines changed

7 files changed

+124
-24
lines changed

‎pdns/dnsproxy.cc

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,13 @@ void DNSProxy::mainloop(void)
304304
}
305305

306306
DNSProxy::~DNSProxy() {
307-
if (d_sock>-1) closesocket(d_sock);
307+
if (d_sock>-1) {
308+
try {
309+
closesocket(d_sock);
310+
}
311+
catch(const PDNSException& e) {
312+
}
313+
}
314+
308315
d_sock=-1;
309316
}

‎pdns/pdns_recursor.cc

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,13 @@ class UDPClientSocks
388388
if(connect(*fd, (struct sockaddr*)(&toaddr), toaddr.getSocklen()) < 0) {
389389
int err = errno;
390390
// returnSocket(*fd);
391-
closesocket(*fd);
391+
try {
392+
closesocket(*fd);
393+
}
394+
catch(const PDNSException& e) {
395+
L<<Logger::Error<<"Error closing UDP socket after connect() failed: "<<e.reason<<endl;
396+
}
397+
392398
if(err==ENETUNREACH) // Seth "My Interfaces Are Like A Yo Yo" Arnold special
393399
return -2;
394400
return -1;
@@ -420,7 +426,12 @@ class UDPClientSocks
420426
catch(FDMultiplexerException& e) {
421427
// we sometimes return a socket that has not yet been assigned to t_fdm
422428
}
423-
closesocket(*i);
429+
try {
430+
closesocket(*i);
431+
}
432+
catch(const PDNSException& e) {
433+
L<<Logger::Error<<"Error closing returned UDP socket: "<<e.reason<<endl;
434+
}
424435

425436
d_socks.erase(i++);
426437
--d_numsocks;
@@ -571,8 +582,14 @@ TCPConnection::TCPConnection(int fd, const ComboAddress& addr) : d_remote(addr),
571582

572583
TCPConnection::~TCPConnection()
573584
{
574-
if(closesocket(d_fd) < 0)
575-
unixDie("closing socket for TCPConnection");
585+
try {
586+
if(closesocket(d_fd) < 0)
587+
L<<Logger::Error<<"Error closing socket for TCPConnection"<<endl;
588+
}
589+
catch(const PDNSException& e) {
590+
L<<Logger::Error<<"Error closing TCPConnection socket: "<<e.reason<<endl;
591+
}
592+
576593
if(t_tcpClientCounts->count(d_remote) && !(*t_tcpClientCounts)[d_remote]--)
577594
t_tcpClientCounts->erase(d_remote);
578595
--s_currentConnections;
@@ -1408,7 +1425,12 @@ void handleNewTCPQuestion(int fd, FDMultiplexer::funcparam_t& )
14081425
if(newsock>=0) {
14091426
if(MT->numProcesses() > g_maxMThreads) {
14101427
g_stats.overCapacityDrops++;
1411-
closesocket(newsock);
1428+
try {
1429+
closesocket(newsock);
1430+
}
1431+
catch(const PDNSException& e) {
1432+
L<<Logger::Error<<"Error closing TCP socket after an over capacity drop: "<<e.reason<<endl;
1433+
}
14121434
return;
14131435
}
14141436

@@ -1419,12 +1441,22 @@ void handleNewTCPQuestion(int fd, FDMultiplexer::funcparam_t& )
14191441
L<<Logger::Error<<"["<<MT->getTid()<<"] dropping TCP query from "<<addr.toString()<<", address not matched by allow-from"<<endl;
14201442

14211443
g_stats.unauthorizedTCP++;
1422-
closesocket(newsock);
1444+
try {
1445+
closesocket(newsock);
1446+
}
1447+
catch(const PDNSException& e) {
1448+
L<<Logger::Error<<"Error closing TCP socket after an ACL drop: "<<e.reason<<endl;
1449+
}
14231450
return;
14241451
}
14251452
if(g_maxTCPPerClient && t_tcpClientCounts->count(addr) && (*t_tcpClientCounts)[addr] >= g_maxTCPPerClient) {
14261453
g_stats.tcpClientOverflow++;
1427-
closesocket(newsock); // don't call TCPConnection::closeAndCleanup here - did not enter it in the counts yet!
1454+
try {
1455+
closesocket(newsock); // don't call TCPConnection::closeAndCleanup here - did not enter it in the counts yet!
1456+
}
1457+
catch(const PDNSException& e) {
1458+
L<<Logger::Error<<"Error closing TCP socket after an overflow drop: "<<e.reason<<endl;
1459+
}
14281460
return;
14291461
}
14301462

‎pdns/resolver.cc

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -516,8 +516,14 @@ void AXFRRetriever::connect()
516516
int err;
517517

518518
if((err=::connect(d_sock,(struct sockaddr*)&d_remote, d_remote.getSocklen()))<0 && errno!=EINPROGRESS) {
519-
closesocket(d_sock);
520-
d_sock=-1;
519+
try {
520+
closesocket(d_sock);
521+
}
522+
catch(const PDNSException& e) {
523+
d_sock=-1;
524+
throw ResolverException("Error closing AXFR socket after connect() failed: "+e.reason);
525+
}
526+
521527
throw ResolverException("connect: "+stringerror());
522528
}
523529

@@ -527,7 +533,14 @@ void AXFRRetriever::connect()
527533
err=waitForRWData(d_sock, false, 10, 0); // wait for writeability
528534

529535
if(!err) {
530-
closesocket(d_sock); // timeout
536+
try {
537+
closesocket(d_sock); // timeout
538+
}
539+
catch(const PDNSException& e) {
540+
d_sock=-1;
541+
throw ResolverException("Error closing AXFR socket after timeout: "+e.reason);
542+
}
543+
531544
d_sock=-1;
532545
errno=ETIMEDOUT;
533546

‎pdns/rfc2136handler.cc

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -603,7 +603,12 @@ int PacketHandler::forwardPacket(const string &msgPrefix, DNSPacket *p, DomainIn
603603

604604
if( connect(sock, (struct sockaddr*)&remote, remote.getSocklen()) < 0 ) {
605605
L<<Logger::Error<<msgPrefix<<"Failed to connect to "<<remote.toStringWithPort()<<": "<<stringerror()<<endl;
606-
closesocket(sock);
606+
try {
607+
closesocket(sock);
608+
}
609+
catch(const PDNSException& e) {
610+
L<<Logger::Error<<"Error closing master forwarding socket after connect() failed: "<<e.reason<<endl;
611+
}
607612
continue;
608613
}
609614

@@ -615,19 +620,34 @@ int PacketHandler::forwardPacket(const string &msgPrefix, DNSPacket *p, DomainIn
615620
buffer.append(forwardPacket.getString());
616621
if(write(sock, buffer.c_str(), buffer.length()) < 0) {
617622
L<<Logger::Error<<msgPrefix<<"Unable to forward update message to "<<remote.toStringWithPort()<<", error:"<<stringerror()<<endl;
618-
closesocket(sock);
623+
try {
624+
closesocket(sock);
625+
}
626+
catch(const PDNSException& e) {
627+
L<<Logger::Error<<"Error closing master forwarding socket after write() failed: "<<e.reason<<endl;
628+
}
619629
continue;
620630
}
621631

622632
int res = waitForData(sock, 10, 0);
623633
if (!res) {
624634
L<<Logger::Error<<msgPrefix<<"Timeout waiting for reply from master at "<<remote.toStringWithPort()<<endl;
625-
closesocket(sock);
635+
try {
636+
closesocket(sock);
637+
}
638+
catch(const PDNSException& e) {
639+
L<<Logger::Error<<"Error closing master forwarding socket after a timeout occured: "<<e.reason<<endl;
640+
}
626641
continue;
627642
}
628643
if (res < 0) {
629644
L<<Logger::Error<<msgPrefix<<"Error waiting for answer from master at "<<remote.toStringWithPort()<<", error:"<<stringerror()<<endl;
630-
closesocket(sock);
645+
try {
646+
closesocket(sock);
647+
}
648+
catch(const PDNSException& e) {
649+
L<<Logger::Error<<"Error closing master forwarding socket after an error occured: "<<e.reason<<endl;
650+
}
631651
continue;
632652
}
633653

@@ -636,7 +656,12 @@ int PacketHandler::forwardPacket(const string &msgPrefix, DNSPacket *p, DomainIn
636656
recvRes = recv(sock, &lenBuf, sizeof(lenBuf), 0);
637657
if (recvRes < 0) {
638658
L<<Logger::Error<<msgPrefix<<"Could not receive data (length) from master at "<<remote.toStringWithPort()<<", error:"<<stringerror()<<endl;
639-
closesocket(sock);
659+
try {
660+
closesocket(sock);
661+
}
662+
catch(const PDNSException& e) {
663+
L<<Logger::Error<<"Error closing master forwarding socket after recv() failed: "<<e.reason<<endl;
664+
}
640665
continue;
641666
}
642667
int packetLen = lenBuf[0]*256+lenBuf[1];
@@ -646,10 +671,20 @@ int PacketHandler::forwardPacket(const string &msgPrefix, DNSPacket *p, DomainIn
646671
recvRes = recv(sock, &buf, packetLen, 0);
647672
if (recvRes < 0) {
648673
L<<Logger::Error<<msgPrefix<<"Could not receive data (dnspacket) from master at "<<remote.toStringWithPort()<<", error:"<<stringerror()<<endl;
649-
closesocket(sock);
674+
try {
675+
closesocket(sock);
676+
}
677+
catch(const PDNSException& e) {
678+
L<<Logger::Error<<"Error closing master forwarding socket after recv() failed: "<<e.reason<<endl;
679+
}
650680
continue;
651681
}
652-
closesocket(sock);
682+
try {
683+
closesocket(sock);
684+
}
685+
catch(const PDNSException& e) {
686+
L<<Logger::Error<<"Error closing master forwarding socket: "<<e.reason<<endl;
687+
}
653688

654689
try {
655690
MOADNSParser mdp(false, buf, recvRes);

‎pdns/sstuff.hh

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,12 @@ public:
7878

7979
~Socket()
8080
{
81-
closesocket(d_socket);
81+
try {
82+
closesocket(d_socket);
83+
}
84+
catch(const PDNSException& e) {
85+
}
86+
8287
delete[] d_buffer;
8388
}
8489

‎pdns/tcpreceiver.cc

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,12 @@ void *TCPNameserver::doConnection(void *data)
259259
if(getpeername(fd, (struct sockaddr *)&remote, &remotelen) < 0) {
260260
L<<Logger::Warning<<"Received question from socket which had no remote address, dropping ("<<stringerror()<<")"<<endl;
261261
d_connectionroom_sem->post();
262-
closesocket(fd);
262+
try {
263+
closesocket(fd);
264+
}
265+
catch(const PDNSException& e) {
266+
L<<Logger::Error<<"Error closing TCP socket: "<<e.reason<<endl;
267+
}
263268
return 0;
264269
}
265270

@@ -386,7 +391,13 @@ void *TCPNameserver::doConnection(void *data)
386391
L << Logger::Error << "TCP Connection Thread caught unknown exception." << endl;
387392
}
388393
d_connectionroom_sem->post();
389-
closesocket(fd);
394+
395+
try {
396+
closesocket(fd);
397+
}
398+
catch(const PDNSException& e) {
399+
L<<Logger::Error<<"Error closing TCP socket: "<<e.reason<<endl;
400+
}
390401

391402
return 0;
392403
}

‎pdns/utility.hh

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,6 @@ public:
9292
typedef int sock_t;
9393
typedef ::socklen_t socklen_t;
9494

95-
//! Closes a socket.
96-
static int closesocket( sock_t socket );
97-
9895
//! Connect with timeout
9996
// Returns:
10097
// > 0 on success

0 commit comments

Comments
 (0)