From: Christian Hofstaedtler <christian@hofstaedtler.name>
Date: Wed, 28 Aug 2013 11:18:53 +0000 (+0200)
Subject: Introduce makeUNsockaddr and have everybody use it
X-Git-Tag: rec-3.6.0-rc1~473^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F976%2Fhead;p=thirdparty%2Fpdns.git

Introduce makeUNsockaddr and have everybody use it

makeUNsockaddr does the length check in a central place, and calling
code should be a bit more readable now.
---

diff --git a/modules/pipebackend/coprocess.cc b/modules/pipebackend/coprocess.cc
index 110c5033d5..8357781209 100644
--- a/modules/pipebackend/coprocess.cc
+++ b/modules/pipebackend/coprocess.cc
@@ -171,10 +171,8 @@ UnixRemote::UnixRemote(const string& path, int timeout)
     throw PDNSException("Unable to create UNIX domain socket: "+string(strerror(errno)));
 
   struct sockaddr_un remote;
-  memset(&remote, 0, sizeof(remote));
-  remote.sun_family = AF_UNIX;
-  memset(remote.sun_path, 0, sizeof(remote.sun_path));
-  path.copy(remote.sun_path, sizeof(remote.sun_path), 0);
+  if (makeUNsockaddr(path, &remote))
+    throw PDNSException("Unable to create UNIX domain socket: Path '"+path+"' is not a valid UNIX socket path.");
 
   // fcntl(fd, F_SETFL, O_NONBLOCK, &sock);
 
diff --git a/modules/remotebackend/unixconnector.cc b/modules/remotebackend/unixconnector.cc
index e0871a7412..1b68353703 100644
--- a/modules/remotebackend/unixconnector.cc
+++ b/modules/remotebackend/unixconnector.cc
@@ -133,9 +133,12 @@ void UnixsocketConnector::reconnect() {
       L<<Logger::Error<<"Cannot create socket: " << strerror(errno) << std::endl;;
       return;
    }
-   sock.sun_family = AF_UNIX;
-   memset(sock.sun_path, 0, UNIX_PATH_MAX);
-   path.copy(sock.sun_path, UNIX_PATH_MAX, 0);
+
+   if (makeUNsockaddr(path, &sock)) {
+      L<<Logger::Error<<"Unable to create UNIX domain socket: Path '"<<path<<"' is not a valid UNIX socket path."<<std::endl;
+      return;
+   }
+
    if (fcntl(fd, F_SETFL, O_NONBLOCK, &fd)) {
       connected = false;
       L<<Logger::Error<<"Cannot manipulate socket: " << strerror(errno) << std::endl;;
diff --git a/pdns/dynlistener.cc b/pdns/dynlistener.cc
index 117c8b5e8b..0e80ab15bd 100644
--- a/pdns/dynlistener.cc
+++ b/pdns/dynlistener.cc
@@ -94,13 +94,10 @@ bool DynListener::testLive(const string& fname)
     return false;
   }
 
-  memset(&addr, 0, sizeof(addr));
-  addr.sun_family = AF_UNIX;
-  if(fname.length()+1 > sizeof(addr.sun_path)) {
-    L<<Logger::Critical<<"Unable to open controlsocket, path '"<<fname<<"' too long."<<endl;
+  if (makeUNsockaddr(fname, &addr)) {
+    L<<Logger::Critical<<"Unable to open controlsocket, path '"<<fname<<"' is not a valid UNIX socket path."<<endl;
     exit(1);
   }
-  strcpy(addr.sun_path, fname.c_str());
 
   int status = connect(fd, (struct sockaddr*)&addr, sizeof(addr));
   close(fd);
@@ -120,13 +117,10 @@ void DynListener::listenOnUnixDomain(const string& fname)
   }
 
   struct sockaddr_un local;
-  memset(&local,0,sizeof(local));
-  local.sun_family=AF_UNIX;
-  if(fname.length()+1 > sizeof(local.sun_path)) {
-    L<<Logger::Critical<<"Unable to bind to controlsocket, path '"<<fname<<"' too long."<<endl;
+  if (makeUNsockaddr(fname, &local)) {
+    L<<Logger::Critical<<"Unable to bind to controlsocket, path '"<<fname<<"' is not a valid UNIX socket path."<<endl;
     exit(1);
   }
-  strcpy(local.sun_path, fname.c_str());
   
   createSocketAndBind(AF_UNIX, (struct sockaddr*)& local, sizeof(local));
   d_socketname=fname;
diff --git a/pdns/dynlistener.hh b/pdns/dynlistener.hh
index 827ecd2703..bcec327b3e 100644
--- a/pdns/dynlistener.hh
+++ b/pdns/dynlistener.hh
@@ -65,12 +65,10 @@ private:
   void listenOnTCP(const ComboAddress&);
   void createSocketAndBind(int family, struct sockaddr*local, size_t len);
 
-#ifndef WIN32
-  struct sockaddr_un d_remote;
-#else
+#ifdef WIN32
   HANDLE m_pipeHandle;
 #endif // WIN32
-  
+
   Utility::socklen_t d_addrlen;
   NetmaskGroup d_tcprange;
   int d_s;
diff --git a/pdns/dynmessenger.cc b/pdns/dynmessenger.cc
index eced8f229b..ba746bf1a9 100644
--- a/pdns/dynmessenger.cc
+++ b/pdns/dynmessenger.cc
@@ -33,40 +33,38 @@ DynMessenger::DynMessenger(const string &localdir, const string &fname)
   if(d_s<0) {
     throw PDNSException(string("socket")+strerror(errno));
   }
-  
-  memset(&d_local,0,sizeof(d_local));
 
   string localname=localdir;
 
   localname+="/lsockXXXXXX";
-  d_local.sun_family=AF_UNIX;
-  strcpy(d_local.sun_path,localname.c_str());
+  if (makeUNsockaddr(localname, &d_local))
+    throw PDNSException("Unable to bind to local temporary file, path '"+localname+"' is not a valid UNIX socket path.");
 
   if(mkstemp(d_local.sun_path)<0)
-    throw PDNSException("Unable to generate local temporary file: "+string(strerror(errno)));
+    throw PDNSException("Unable to generate local temporary file: "+stringerror());
   
   unlink(d_local.sun_path);
-  
-  if(bind(d_s, (sockaddr*)&d_local,sizeof(d_local))<0) {
-    unlink(d_local.sun_path);
-    throw PDNSException("Unable to bind to local temporary file: "+string(strerror(errno)));
-  }
-  
-  if(chmod(d_local.sun_path,0666)<0) { // make sure that pdns can reply!
-    unlink(d_local.sun_path);
-    perror("fchmod");
-    exit(1);
-  }
 
-  memset(&d_remote,0,sizeof(d_remote));
-  
-  d_remote.sun_family=AF_UNIX;
-  strcpy(d_remote.sun_path,fname.c_str());
-  if(connect(d_s,(sockaddr*)&d_remote,sizeof(d_remote))<0) {
+  try {
+    if(bind(d_s, (sockaddr*)&d_local,sizeof(d_local))<0)
+      throw PDNSException("Unable to bind to local temporary file: "+stringerror());
+
+    // make sure that pdns can reply!
+    if(chmod(d_local.sun_path,0666)<0)
+      throw PDNSException("Unable to chmod local temporary file: "+stringerror());
+
+    if(makeUNsockaddr(fname, &d_remote))
+      throw PDNSException("Unable to connect to remote '"+fname+"': Path is not a valid UNIX socket path.");
+
+    if(connect(d_s,(sockaddr*)&d_remote,sizeof(d_remote))<0)
+      throw PDNSException("Unable to connect to remote '"+fname+"': "+stringerror());
+
+  } catch(...) {
+    close(d_s);
+    d_s=-1;
     unlink(d_local.sun_path);
-    throw PDNSException("Unable to connect to remote '"+fname+"': "+string(strerror(errno)));
+    throw;
   }
-  
 }
 
 DynMessenger::DynMessenger(const ComboAddress& remote, const string &secret)
@@ -81,6 +79,7 @@ DynMessenger::DynMessenger(const ComboAddress& remote, const string &secret)
   
   if(connect(d_s, (sockaddr*)&remote, remote.getSocklen())<0) {
     close(d_s);
+    d_s=-1;
     throw PDNSException("Unable to connect to remote '"+remote.toStringWithPort()+"': "+string(strerror(errno)));
   }
 
@@ -90,9 +89,10 @@ DynMessenger::DynMessenger(const ComboAddress& remote, const string &secret)
 
 DynMessenger::~DynMessenger()
 {
+  if (d_s > 0)
+    close(d_s);
   if(*d_local.sun_path && unlink(d_local.sun_path)<0)
     cerr<<"Warning: unable to unlink local unix domain endpoint: "<<strerror(errno)<<endl;
-  close(d_s);
 }   
 
 int DynMessenger::send(const string &msg) const
diff --git a/pdns/misc.cc b/pdns/misc.cc
index 866711052b..874a9561da 100644
--- a/pdns/misc.cc
+++ b/pdns/misc.cc
@@ -23,6 +23,7 @@
 #include <sys/time.h>
 #include <time.h>
 #include <netinet/in.h>
+#include <sys/un.h>
 #include <unistd.h>
 #endif // WIN32
 
@@ -713,7 +714,7 @@ int makeIPv6sockaddr(const std::string& addr, struct sockaddr_in6* ret)
   return 0;
 }
 
-int makeIPv4sockaddr(const string &str, struct sockaddr_in* ret)
+int makeIPv4sockaddr(const std::string& str, struct sockaddr_in* ret)
 {
   if(str.empty()) {
     return -1;
@@ -744,6 +745,19 @@ int makeIPv4sockaddr(const string &str, struct sockaddr_in* ret)
   return -1;
 }
 
+int makeUNsockaddr(const std::string& path, struct sockaddr_un* ret)
+{
+  if (path.empty())
+    return -1;
+
+  memset(ret, 0, sizeof(struct sockaddr_un));
+  ret->sun_family = AF_UNIX;
+  if (path.length() >= sizeof(ret->sun_path))
+    return -1;
+
+  path.copy(ret->sun_path, sizeof(ret->sun_path), 0);
+  return 0;
+}
 
 //! read a line of text from a FILE* to a std::string, returns false on 'no data'
 bool stringfgets(FILE* fp, std::string& line)
diff --git a/pdns/misc.hh b/pdns/misc.hh
index 1742d695a7..505db2982b 100644
--- a/pdns/misc.hh
+++ b/pdns/misc.hh
@@ -476,7 +476,8 @@ string makeRelative(const std::string& fqdn, const std::string& zone);
 string labelReverse(const std::string& qname);
 std::string dotConcat(const std::string& a, const std::string &b);
 int makeIPv6sockaddr(const std::string& addr, struct sockaddr_in6* ret);
-int makeIPv4sockaddr(const string &str, struct sockaddr_in* ret);
+int makeIPv4sockaddr(const std::string& str, struct sockaddr_in* ret);
+int makeUNsockaddr(const std::string& path, struct sockaddr_un* ret);
 bool stringfgets(FILE* fp, std::string& line);
 
 template<typename Index>
diff --git a/pdns/rec_channel.cc b/pdns/rec_channel.cc
index 3c36438517..1b28b1edcd 100644
--- a/pdns/rec_channel.cc
+++ b/pdns/rec_channel.cc
@@ -33,24 +33,21 @@ int RecursorControlChannel::listen(const string& fname)
   Utility::setCloseOnExec(d_fd);
 
   if(d_fd < 0) 
-    throw PDNSException("Creating UNIX domain socket: "+string(strerror(errno)));
+    throw PDNSException("Creating UNIX domain socket: "+stringerror());
   
   int tmp=1;
   if(setsockopt(d_fd, SOL_SOCKET, SO_REUSEADDR,(char*)&tmp,sizeof tmp)<0)
-    throw PDNSException(string("Setsockopt failed: ")+strerror(errno));
+    throw PDNSException("Setsockopt failed: "+stringerror());
   
   int err=unlink(fname.c_str());
   if(err < 0 && errno!=ENOENT)
-    throw PDNSException("Can't remove (previous) controlsocket '"+fname+"': "+string(strerror(errno)) + " (try --socket-dir)");
+    throw PDNSException("Can't remove (previous) controlsocket '"+fname+"': "+stringerror() + " (try --socket-dir)");
 
-  memset(&d_local,0,sizeof(d_local));
-  d_local.sun_family=AF_UNIX;
-  if(fname.length()+1 > sizeof(d_local.sun_path))
-    throw PDNSException("Unable to bind to controlsocket, path '"+fname+"' too long.");
-  strcpy(d_local.sun_path, fname.c_str());
+  if(makeUNsockaddr(fname, &d_local))
+    throw PDNSException("Unable to bind to controlsocket, path '"+fname+"' is not a valid UNIX socket path.");
     
   if(bind(d_fd, (sockaddr*)&d_local,sizeof(d_local))<0) 
-    throw PDNSException("Unable to bind to controlsocket '"+fname+"': "+string(strerror(errno)));
+    throw PDNSException("Unable to bind to controlsocket '"+fname+"': "+stringerror());
 
   return d_fd;
 }
@@ -64,49 +61,41 @@ void RecursorControlChannel::connect(const string& path, const string& fname)
 
   if(d_fd < 0) 
     throw PDNSException("Creating UNIX domain socket: "+string(strerror(errno)));
+
+  try {
+    int tmp=1;
+    if(setsockopt(d_fd, SOL_SOCKET, SO_REUSEADDR,(char*)&tmp,sizeof tmp)<0)
+      throw PDNSException("Setsockopt failed: "+stringerror());
   
-  int tmp=1;
-  if(setsockopt(d_fd, SOL_SOCKET, SO_REUSEADDR,(char*)&tmp,sizeof tmp)<0) {
-    close(d_fd);
-    d_fd=-1;
-    throw PDNSException(string("Setsockopt failed: ")+strerror(errno));
-  }
-  
-  string localname=path+"/lsockXXXXXX";
-  strcpy(d_local.sun_path, localname.c_str());
+    string localname=path+"/lsockXXXXXX";
+    if (makeUNsockaddr(localname, &d_local))
+      throw PDNSException("Unable to bind to local temporary file, path '"+localname+"' is not a valid UNIX socket path.");
 
-  if(mkstemp(d_local.sun_path) < 0) {
-    close(d_fd);
-    d_fd=-1;
-    d_local.sun_path[0]=0;
-    throw PDNSException("Unable to generate local temporary file in directory '"+path+"': "+string(strerror(errno)));
-  }
+    if(mkstemp(d_local.sun_path) < 0)
+      throw PDNSException("Unable to generate local temporary file in directory '"+path+"': "+stringerror());
 
-  d_local.sun_family=AF_UNIX;
+    int err=unlink(d_local.sun_path);
+    if(err < 0 && errno!=ENOENT)
+      throw PDNSException("Unable to remove local controlsocket: "+stringerror());
 
-  int err=unlink(d_local.sun_path);
-  if(err < 0 && errno!=ENOENT)
-    throw PDNSException("Unable to remove local controlsocket: "+string(strerror(errno)));
+    if(bind(d_fd, (sockaddr*)&d_local,sizeof(d_local))<0)
+      throw PDNSException("Unable to bind to local temporary file: "+stringerror());
 
-  if(bind(d_fd, (sockaddr*)&d_local,sizeof(d_local))<0) {
-    unlink(d_local.sun_path);
-    close(d_fd);
-    d_fd=-1;
-    throw PDNSException("Unable to bind to local temporary file: "+string(strerror(errno)));
-  }
+    if(chmod(d_local.sun_path,0666)<0) // make sure that pdns can reply!
+      throw PDNSException("Unable to chmod local temporary socket: "+stringerror());
 
-  if(chmod(d_local.sun_path,0666)<0) { // make sure that pdns can reply!
-    unlink(d_local.sun_path);
-    throw PDNSException("Unable to chmnod local temporary socket: "+string(strerror(errno)));
-  }
+    string remotename=path+"/"+fname;
+    if (makeUNsockaddr(remotename, &remote))
+      throw PDNSException("Unable to connect to controlsocket, path '"+remotename+"' is not a valid UNIX socket path.");
 
-  memset(&remote,0,sizeof(remote));
-  
-  remote.sun_family=AF_UNIX;
-  strcpy(remote.sun_path,(path+"/"+fname).c_str());
-  if(::connect(d_fd, (sockaddr*)&remote, sizeof(remote)) < 0) {
-    unlink(d_local.sun_path);
-    throw PDNSException("Unable to connect to remote '"+string(remote.sun_path)+"': "+string(strerror(errno)));
+    if(::connect(d_fd, (sockaddr*)&remote, sizeof(remote)) < 0)
+      throw PDNSException("Unable to connect to remote '"+string(remote.sun_path)+"': "+stringerror());
+
+  } catch (...) {
+    close(d_fd);
+    d_fd=-1;
+    d_local.sun_path[0]=0;
+    throw;
   }
 }