Issue #20: Make abstract sockets work

Finish the work started by Daniel in 2012, by using abstract local
AF_UNIX sockets on platforms that support them (Linux, Android).

Fix all sorts of bugs and issues that prevented the existing MDP and
Monitor client and server code from working with abstract socket names.

Makefile.in

@@ -29,6 +29,7 @@ MONITORCLIENTSRCS=conf.c \
 	monitor-client.c \
         instance.c \
 	net.c \
+	socket.c \
 	str.c \
 	strbuf.c \
 	strbuf_helpers.c \
@@ -46,6 +47,7 @@ MDPCLIENTSRCS=conf.c \
 	mdp_client.c \
         instance.c \
 	net.c \
+	socket.c \
 	str.c \
 	strbuf.c \
 	strbuf_helpers.c \

configure.in

@@ -85,7 +85,6 @@ AC_CHECK_HEADERS(
     sys/ucred.h \
     poll.h \
     netdb.h \
-    linux/if.h \
     linux/ioctl.h \
     linux/netlink.h \
     linux/rtnetlink.h \
@@ -99,8 +98,16 @@ AC_CHECK_HEADERS(
     sys/filio.h \
     sys/endian.h \
     sys/byteorder.h \
-    sys/sockio.h
+    sys/sockio.h \
+    sys/socket.h
 )
+AC_CHECK_HEADERS(
+    linux/if.h
+,,, [
+#ifdef HAVE_SYS_SOCKET_H
+#include <sys/socket.h>
+#endif
+])
 
 dnl Check for ALSA
 AC_CHECK_HEADER([alsa/asoundlib.h], [have_alsa=1], [have_alsa=0])
@@ -108,12 +115,7 @@ AS_IF([test x"$have_alsa" = "x1"], [AC_DEFINE([HAVE_ALSA_ASOUNDLIB_H])])
 AS_IF([test x"$have_alsa" = "x1"], [AC_SUBST(HAVE_ALSA,1)], [AC_SUBST(HAVE_ALSA,0)])
 
 dnl Lazy way of checking for Linux
-AC_CHECK_HEADER([sys/socket.h])
-AC_CHECK_HEADER([linux/if.h], [AC_DEFINE([USE_ABSTRACT_NAMESPACE])],, [
-#ifdef HAVE_SYS_SOCKET_H
-#include <sys/socket.h>
-#endif
-])
+AS_IF([test "x$ac_cv_header_linux_if_h" = xyes], [AC_DEFINE([USE_ABSTRACT_NAMESPACE])])
 
 AC_CHECK_LIB(m,sqrtf,[LDFLAGS="$LDFLAGS -lm"])
 AC_CHECK_LIB(nsl,callrpc,[LDFLAGS="$LDFLAGS -lnsl"])

mdp_client.c

@@ -19,38 +19,35 @@
 #include <sys/stat.h>
 #include "serval.h"
 #include "conf.h"
+#include "log.h"
 #include "str.h"
 #include "strbuf.h"
+#include "strbuf_helpers.h"
 #include "overlay_buffer.h"
 #include "overlay_address.h"
 #include "overlay_packet.h"
 #include "mdp_client.h"
 
-int mdp_client_socket=-1;
-int overlay_mdp_send(overlay_mdp_frame *mdp,int flags,int timeout_ms)
+int mdp_client_socket = -1;
+
+int overlay_mdp_send(overlay_mdp_frame *mdp, int flags, int timeout_ms)
 {
-  int len=4;
-  
-  if (mdp_client_socket==-1) 
-    if (overlay_mdp_client_init() != 0)
-      return -1;
-  
-  /* Minimise frame length to save work and prevent accidental disclosure of
-   memory contents. */
-  len=overlay_mdp_relevant_bytes(mdp);
-  if (len<0) return WHY("MDP frame invalid (could not compute length)");
-  
-  /* Construct name of socket to send to. */
-  struct sockaddr_un name;
-  name.sun_family = AF_UNIX;
-  if (!FORM_SERVAL_INSTANCE_PATH(name.sun_path, "mdp.socket"))
+  if (mdp_client_socket == -1 && overlay_mdp_client_init() == -1)
     return -1;
-  
+  // Minimise frame length to save work and prevent accidental disclosure of memory contents.
+  int len = overlay_mdp_relevant_bytes(mdp);
+  if (len < 0)
+    return WHY("MDP frame invalid (could not compute length)");
+  /* Construct name of socket to send to. */
+  struct sockaddr_un addr;
+  socklen_t addrlen;
+  if (socket_setname(&addr, &addrlen, "mdp.socket") == -1)
+    return -1;
+  // Send to that socket
   set_nonblock(mdp_client_socket);
-  int result=sendto(mdp_client_socket, mdp, len, 0,
-		    (struct sockaddr *)&name, sizeof(struct sockaddr_un));
+  int result = sendto(mdp_client_socket, mdp, len, 0, (struct sockaddr *)&addr, addrlen);
   set_block(mdp_client_socket);
-  if (result<0) {
+  if (result == -1) {
     mdp->packetTypeAndFlags=MDP_ERROR;
     mdp->error.error=1;
     snprintf(mdp->error.message,128,"Error sending frame to MDP server.");
@@ -91,76 +88,46 @@ int overlay_mdp_send(overlay_mdp_frame *mdp,int flags,int timeout_ms)
 
 int overlay_mdp_client_init()
 {
-  char overlay_mdp_client_socket_path[1024];
-  int overlay_mdp_client_socket_path_len=-1;
-  if (mdp_client_socket==-1) {
-    /* Open socket to MDP server (thus connection is always local) */
-    if (0) WHY("Use of abstract name space socket for Linux not implemented");
-    
-    mdp_client_socket = socket(AF_UNIX, SOCK_DGRAM, 0);
-    if (mdp_client_socket < 0) {
-      WHY_perror("socket");
-      return WHY("Could not open socket to MDP server");
-    }
-    
-    /* We must bind to a temporary file name */
-    struct sockaddr_un name;
-    unsigned int random_value;
-    if (urandombytes((unsigned char *)&random_value,sizeof(int)))
+  if (mdp_client_socket == -1) {
+    /* Create local per-client socket to MDP server (connection is always local) */
+    struct sockaddr_un addr;
+    socklen_t addrlen;
+    uint32_t random_value;
+    if (urandombytes((unsigned char *)&random_value, sizeof random_value) == -1)
       return WHY("urandombytes() failed");
-    name.sun_family = AF_UNIX;
-    if (overlay_mdp_client_socket_path_len==-1) {
-      char fmt[1024];
-      if (!FORM_SERVAL_INSTANCE_PATH(fmt, "mdp-client-%d-%08x.socket"))
-	return WHY("Could not form MDP client socket name");
-      snprintf(overlay_mdp_client_socket_path,1024,fmt,getpid(),random_value);
-      overlay_mdp_client_socket_path_len=strlen(overlay_mdp_client_socket_path)+1;
-      if(config.debug.io) DEBUGF("MDP client socket name='%s'",overlay_mdp_client_socket_path);
+    if (socket_setname(&addr, &addrlen, "mdp.client.%u.%08lx.socket", getpid(), (unsigned long)random_value) == -1)
+      return -1;
+    if ((mdp_client_socket = esocket(AF_UNIX, SOCK_DGRAM, 0)) == -1)
+      return -1;
+    if (socket_bind(mdp_client_socket, (struct sockaddr *)&addr, addrlen) == -1) {
+      close(mdp_client_socket);
+      mdp_client_socket = -1;
+      return -1;
     }
-    if (overlay_mdp_client_socket_path_len > sizeof(name.sun_path) - 1)
-      FATALF("MDP socket path too long (%d > %d)", overlay_mdp_client_socket_path_len, (int)sizeof(name.sun_path) - 1);
-    
-    bcopy(overlay_mdp_client_socket_path,name.sun_path,
-	  overlay_mdp_client_socket_path_len);
-    unlink(name.sun_path);
-    int len = 1 + strlen(name.sun_path) + sizeof(name.sun_family) + 1;
-    int r=bind(mdp_client_socket, (struct sockaddr *)&name, len);
-    if (r) {
-      WHY_perror("bind");
-      return WHY("Could not bind MDP client socket to file name");
-    }
-    
-    int send_buffer_size=128*1024;
-    if (setsockopt(mdp_client_socket, SOL_SOCKET, SO_RCVBUF, 
-		   &send_buffer_size, sizeof(send_buffer_size)) == -1)
-      WARN_perror("setsockopt");
+    socket_set_rcvbufsize(mdp_client_socket, 128 * 1024);
   }
-  
   return 0;
 }
 
 int overlay_mdp_client_done()
 {
-  if (mdp_client_socket!=-1) {
+  if (mdp_client_socket != -1) {
     /* Tell MDP server to release all our bindings */
     overlay_mdp_frame mdp;
-    mdp.packetTypeAndFlags=MDP_GOODBYE;
-    overlay_mdp_send(&mdp,0,0);
-    
-    // get the socket name and unlink it from the filesystem
-    struct sockaddr_un name;
-    socklen_t len=sizeof(name);
-    if (getsockname(mdp_client_socket, (struct sockaddr *)&name, &len))
-      WHY_perror("getsockname()");
-    else if(len>sizeof(sa_family_t) && len<=sizeof(name)){
-      if (unlink(name.sun_path))
-	WHY_perror("unlink()");
+    mdp.packetTypeAndFlags = MDP_GOODBYE;
+    overlay_mdp_send(&mdp, 0, 0);
+    // get the socket name and unlink it from the filesystem if not abstract
+    struct sockaddr_un addr;
+    socklen_t addrlen = sizeof addr;
+    if (getsockname(mdp_client_socket, (struct sockaddr *)&addr, &addrlen))
+      WHYF_perror("getsockname(%d)", mdp_client_socket);
+    else if (addrlen > sizeof addr.sun_family && addrlen <= sizeof addr && addr.sun_path[0] != '\0') {
+      if (unlink(addr.sun_path) == -1)
+	WARNF_perror("unlink(%s)", alloca_str_toprint(addr.sun_path));
     }
-    
     close(mdp_client_socket);
+    mdp_client_socket = -1;
   }
-  
-  mdp_client_socket=-1;
   return 0;
 }
 
@@ -186,56 +153,43 @@ int overlay_mdp_client_poll(time_ms_t timeout_ms)
 
 int overlay_mdp_recv(overlay_mdp_frame *mdp, int port, int *ttl) 
 {
-  char mdp_socket_name[101];
-  unsigned char recvaddrbuffer[1024];
-  struct sockaddr *recvaddr=(struct sockaddr *)recvaddrbuffer;
-  unsigned int recvaddrlen=sizeof(recvaddrbuffer);
-  struct sockaddr_un *recvaddr_un;
-  
-  if (!FORM_SERVAL_INSTANCE_PATH(mdp_socket_name, "mdp.socket"))
-    return WHY("Could not find mdp socket");
-  mdp->packetTypeAndFlags=0;
-  
-  /* Check if reply available */
-  set_nonblock(mdp_client_socket);
-  ssize_t len = recvwithttl(mdp_client_socket,(unsigned char *)mdp, sizeof(overlay_mdp_frame),ttl,recvaddr,&recvaddrlen);
-  set_block(mdp_client_socket);
-  
-  recvaddr_un=(struct sockaddr_un *)recvaddr;
-  /* Null terminate received address so that the stat() call below can succeed */
-  if (recvaddrlen<1024) recvaddrbuffer[recvaddrlen]=0;
-  if (len>0) {
-    /* Make sure recvaddr matches who we sent it to */
-    if (strncmp(mdp_socket_name, recvaddr_un->sun_path, sizeof(recvaddr_un->sun_path))) {
-      /* Okay, reply was PROBABLY not from the server, but on OSX if the path
-       has a symlink in it, it is resolved in the reply path, but might not
-       be in the request path (mdp_socket_name), thus we need to stat() and
-       compare inode numbers etc */
-      struct stat sb1,sb2;
-      if (stat(mdp_socket_name,&sb1)) return WHY("stat(mdp_socket_name) failed, so could not verify that reply came from MDP server");
-      if (stat(recvaddr_un->sun_path,&sb2)) return WHY("stat(ra->sun_path) failed, so could not verify that reply came from MDP server");
-      if ((sb1.st_ino!=sb2.st_ino)||(sb1.st_dev!=sb2.st_dev))
-	return WHY("Reply did not come from server");
-    }
-    
-    // silently drop incoming packets for the wrong port number
-    if (port>0 && port != mdp->in.dst.port){
-      WARNF("Ignoring packet for port %d",mdp->in.dst.port);
-      return -1;
-    }
-    
-    int expected_len = overlay_mdp_relevant_bytes(mdp);
-    
-    if (len < expected_len){
-      return WHYF("Expected packet length of %d, received only %lld bytes", expected_len, (long long) len);
-    }
-    
-    /* Valid packet received */
-    return 0;
-  } else 
-  /* no packet received */
+  /* Construct name of socket to receive from. */
+  struct sockaddr_un mdp_addr;
+  socklen_t mdp_addrlen;
+  if (socket_setname(&mdp_addr, &mdp_addrlen, "mdp.socket") == -1)
     return -1;
   
+  /* Check if reply available */
+  struct sockaddr_un recvaddr;
+  socklen_t recvaddrlen = sizeof recvaddr;
+  ssize_t len;
+  mdp->packetTypeAndFlags = 0;
+  set_nonblock(mdp_client_socket);
+  len = recvwithttl(mdp_client_socket, (unsigned char *)mdp, sizeof(overlay_mdp_frame), ttl, (struct sockaddr *)&recvaddr, &recvaddrlen);
+  set_block(mdp_client_socket);
+  if (len <= 0)
+    return -1; // no packet received
+
+  // If the received address overflowed the buffer, then it cannot have come from the server, whose
+  // address always fits within a struct sockaddr_un.
+  if (recvaddrlen > sizeof recvaddr)
+    return WHY("reply did not come from server: address overrun");
+
+  if (cmp_sockaddr((struct sockaddr *)&recvaddr, recvaddrlen, (struct sockaddr *)&mdp_addr, mdp_addrlen) != 0)
+    return WHYF("reply did not come from server: %s", alloca_sockaddr(&recvaddr, recvaddrlen));
+  
+  // silently drop incoming packets for the wrong port number
+  if (port>0 && port != mdp->in.dst.port){
+    WARNF("Ignoring packet for port %d",mdp->in.dst.port);
+    return -1;
+  }
+
+  int expected_len = overlay_mdp_relevant_bytes(mdp);
+  if (len < expected_len)
+    return WHYF("Expected packet length of %d, received only %lld bytes", expected_len, (long long) len);
+  
+  /* Valid packet received */
+  return 0;
 }
 
 // send a request to servald deamon to add a port binding

monitor-client.c

@@ -74,7 +74,7 @@ int monitor_client_open(struct monitor_state **res)
     return WHYF_perror("socket(AF_UNIX, SOCK_STREAM, 0)");
   struct sockaddr_un addr;
   socklen_t addrlen;
-  if (socket_setname(&addr, config.monitor.socket, &addrlen) == -1)
+  if (socket_setname(&addr, &addrlen, "%s", config.monitor.socket) == -1)
     return -1;
   INFOF("Attempting to connect to %s", alloca_sockaddr(&addr, addrlen));
   if (socket_connect(fd, (struct sockaddr*)&addr, addrlen) == -1) {

monitor.c

@@ -82,7 +82,7 @@ int monitor_setup_sockets()
     goto error;
   struct sockaddr_un addr;
   socklen_t addrlen;
-  if (socket_setname(&addr, config.monitor.socket, &addrlen) == -1)
+  if (socket_setname(&addr, &addrlen, "%s", config.monitor.socket) == -1)
     goto error;
   if (socket_bind(sock, (struct sockaddr*)&addr, addrlen) == -1)
     goto error;

net.c

@@ -149,21 +149,20 @@ ssize_t recvwithttl(int sock,unsigned char *buffer, size_t bufferlen,int *ttl,
   if (len == -1 && errno != EAGAIN && errno != EWOULDBLOCK)
     return WHY_perror("recvmsg");
   
-  if (0 && config.debug.packetrx) {
+#if 0
+  if (config.debug.packetrx) {
     DEBUGF("recvmsg returned %d (flags=%d, msg_controllen=%d)", (int) len, msg.msg_flags, (int)msg.msg_controllen);
     dump("received data", buffer, len);
   }
+#endif
   
-  struct cmsghdr *cmsg;
-  if (len>0)
-  {
-    for (cmsg = CMSG_FIRSTHDR(&msg); 
-	 cmsg != NULL; 
-	 cmsg = CMSG_NXTHDR(&msg,cmsg)) {
-      
-      if ((cmsg->cmsg_level == IPPROTO_IP) && 
-	  ((cmsg->cmsg_type == IP_RECVTTL) ||(cmsg->cmsg_type == IP_TTL))
-	  &&(cmsg->cmsg_len) ){
+  if (len > 0) {
+    struct cmsghdr *cmsg;
+    for (cmsg = CMSG_FIRSTHDR(&msg); cmsg != NULL; cmsg = CMSG_NXTHDR(&msg, cmsg)) {
+      if (   cmsg->cmsg_level == IPPROTO_IP
+	  && ((cmsg->cmsg_type == IP_RECVTTL) || (cmsg->cmsg_type == IP_TTL))
+	  && cmsg->cmsg_len
+      ) {
 	if (config.debug.packetrx)
 	  DEBUGF("  TTL (%p) data location resolves to %p", ttl,CMSG_DATA(cmsg));
 	if (CMSG_DATA(cmsg)) {

overlay_mdp.c

@@ -69,7 +69,7 @@ int overlay_mdp_setup_sockets()
   overlay_mdp_clean_socket_files();
 
   if (mdp_sock.poll.fd == -1) {
-    if (socket_setname(&addr, "mdp.socket", &addrlen) == -1)
+    if (socket_setname(&addr, &addrlen, "mdp.socket") == -1)
       return -1;
     if ((mdp_sock.poll.fd = esocket(AF_UNIX, SOCK_DGRAM, 0)) == -1)
       return -1;
@@ -174,13 +174,18 @@ int overlay_mdp_releasebindings(struct sockaddr_un *recvaddr,int recvaddrlen)
 int overlay_mdp_process_bind_request(int sock, struct subscriber *subscriber, int port,
 				     int flags, struct sockaddr_un *recvaddr, int recvaddrlen)
 {
-  int i;
+  if (config.debug.mdprequests) 
+    DEBUGF("Bind request %s:%d",
+	subscriber ? alloca_tohex_sid(subscriber->sid) : "NULL",
+	port
+      );
   
   if (port<=0){
     return WHYF("Port %d cannot be bound", port);
   }
   if (!mdp_bindings_initialised) {
     /* Mark all slots as unused */
+    int i;
     for(i=0;i<MDP_MAX_BINDINGS;i++)
       mdp_bindings[i].port=0;
     mdp_bindings_initialised=1;
@@ -188,24 +193,27 @@ int overlay_mdp_process_bind_request(int sock, struct subscriber *subscriber, in
 
   /* See if binding already exists */
   int free=-1;
-  for(i=0;i<MDP_MAX_BINDINGS;i++) {
-    /* Look for duplicate bindings */
-    if (mdp_bindings[i].port == port && mdp_bindings[i].subscriber == subscriber) {
-      if (mdp_bindings[i].name_len==recvaddrlen &&
-	  !memcmp(mdp_bindings[i].socket_name,recvaddr->sun_path,recvaddrlen)) {
-	// this client already owns this port binding?
-	INFO("Identical binding exists");
-	return 0;
-      }else if(flags&MDP_FORCE){
-	// steal the port binding
-	free=i;
-	break;
-      }else{
-	return WHY("Port already in use");
+  {
+    int i;
+    for(i=0;i<MDP_MAX_BINDINGS;i++) {
+      /* Look for duplicate bindings */
+      if (mdp_bindings[i].port == port && mdp_bindings[i].subscriber == subscriber) {
+	if (mdp_bindings[i].name_len==recvaddrlen &&
+	    !memcmp(mdp_bindings[i].socket_name,recvaddr->sun_path,recvaddrlen)) {
+	  // this client already owns this port binding?
+	  INFO("Identical binding exists");
+	  return 0;
+	}else if(flags&MDP_FORCE){
+	  // steal the port binding
+	  free=i;
+	  break;
+	}else{
+	  return WHY("Port already in use");
+	}
       }
+      /* Look for free slots in case we need one */
+      if ((free==-1)&&(mdp_bindings[i].port==0)) free=i;
     }
-    /* Look for free slots in case we need one */
-    if ((free==-1)&&(mdp_bindings[i].port==0)) free=i;
   }
  
   /* Okay, so no binding exists.  Make one, and return success.
@@ -224,15 +232,12 @@ int overlay_mdp_process_bind_request(int sock, struct subscriber *subscriber, in
     */
     free=random()%MDP_MAX_BINDINGS;
   }
-  if (config.debug.mdprequests) 
-    DEBUGF("Binding %s:%d", subscriber ? alloca_tohex_sid(subscriber->sid) : "NULL", port);
   /* Okay, record binding and report success */
   mdp_bindings[free].port=port;
   mdp_bindings[free].subscriber=subscriber;
   
-  mdp_bindings[free].name_len=recvaddrlen-2;
-  memcpy(mdp_bindings[free].socket_name,recvaddr->sun_path,
-	 mdp_bindings[free].name_len);
+  mdp_bindings[free].name_len = recvaddrlen - sizeof recvaddr->sun_family;
+  memcpy(mdp_bindings[free].socket_name, recvaddr->sun_path, mdp_bindings[free].name_len);
   mdp_bindings[free].binding_time=gettime_ms();
   return 0;
 }
@@ -429,17 +434,15 @@ static int overlay_saw_mdp_frame(struct overlay_frame *frame, overlay_mdp_frame
     
     if (match>-1) {
       struct sockaddr_un addr;
-
-      bcopy(mdp_bindings[match].socket_name,addr.sun_path,mdp_bindings[match].name_len);
-      addr.sun_family=AF_UNIX;
-      errno=0;
+      addr.sun_family = AF_UNIX;
+      bcopy(mdp_bindings[match].socket_name, addr.sun_path, mdp_bindings[match].name_len);
       int len=overlay_mdp_relevant_bytes(mdp);
-      int r=sendto(mdp_sock.poll.fd,mdp,len,0,(struct sockaddr*)&addr,sizeof(addr));
-      if (r==overlay_mdp_relevant_bytes(mdp)) {	
+      socklen_t addrlen = sizeof addr.sun_family + mdp_bindings[match].name_len;
+      int r = sendto(mdp_sock.poll.fd,mdp,len,0,(struct sockaddr*)&addr, addrlen);
+      if (r == overlay_mdp_relevant_bytes(mdp))
 	RETURN(0);
-      }
-      WHY("didn't send mdp packet");
-      if (errno==ENOENT) {
+      WHYF("didn't send mdp packet to %s", alloca_sockaddr(&addr, addrlen));
+      if (r == -1 && errno == ENOENT) {
 	/* far-end of socket has died, so drop binding */
 	INFOF("Closing dead MDP client '%s'",mdp_bindings[match].socket_name);
 	overlay_mdp_releasebindings(&addr,mdp_bindings[match].name_len);
@@ -935,11 +938,14 @@ void overlay_mdp_poll(struct sched_ent *alarm)
 
       switch (mdp_type) {
       case MDP_GOODBYE:
-	if (config.debug.mdprequests) DEBUG("MDP_GOODBYE");
+	if (config.debug.mdprequests)
+	  DEBUGF("MDP_GOODBYE from %s", alloca_sockaddr(recvaddr, recvaddrlen));
 	overlay_mdp_releasebindings(recvaddr_un,recvaddrlen);
 	return;
 	  
       case MDP_ROUTING_TABLE:
+	if (config.debug.mdprequests)
+	  DEBUGF("MDP_ROUTING_TABLE from %s", alloca_sockaddr(recvaddr, recvaddrlen));
 	{
 	  struct routing_state state={
 	    .recvaddr_un=recvaddr_un,
@@ -952,6 +958,8 @@ void overlay_mdp_poll(struct sched_ent *alarm)
 	return;
       
       case MDP_GETADDRS:
+	if (config.debug.mdprequests)
+	  DEBUGF("MDP_GETADDRS from %s", alloca_sockaddr(recvaddr, recvaddrlen));
 	{
 	  overlay_mdp_frame mdpreply;
 	  bzero(&mdpreply, sizeof(overlay_mdp_frame));
@@ -967,7 +975,8 @@ void overlay_mdp_poll(struct sched_ent *alarm)
 	break;
 	  
       case MDP_TX: /* Send payload (and don't treat it as system privileged) */
-	if (config.debug.mdprequests) DEBUG("MDP_TX");
+	if (config.debug.mdprequests)
+	  DEBUGF("MDP_TX from %s", alloca_sockaddr(recvaddr, recvaddrlen));
 	  
 	// Dont allow mdp clients to send very high priority payloads
 	if (mdp->out.queue<=OQ_MESH_MANAGEMENT)
@@ -977,9 +986,9 @@ void overlay_mdp_poll(struct sched_ent *alarm)
 	break;
 	  
       case MDP_BIND: /* Bind to port */
+	if (config.debug.mdprequests)
+	  DEBUGF("MDP_BIND from %s", alloca_sockaddr(recvaddr, recvaddrlen));
 	{
-	  if (config.debug.mdprequests) DEBUG("MDP_BIND");
-	  
 	  struct subscriber *subscriber=NULL;
 	  /* Make sure source address is either all zeros (listen on all), or a valid
 	   local address */
@@ -1005,6 +1014,8 @@ void overlay_mdp_poll(struct sched_ent *alarm)
 	break;
 	  
       case MDP_SCAN:
+	if (config.debug.mdprequests)
+	  DEBUGF("MDP_SCAN from %s", alloca_sockaddr(recvaddr, recvaddrlen));
 	{
 	  struct overlay_mdp_scan *scan = (struct overlay_mdp_scan *)&mdp->raw;
 	  time_ms_t start=gettime_ms();
@@ -1056,7 +1067,7 @@ void overlay_mdp_poll(struct sched_ent *alarm)
 	
       default:
 	/* Client is not allowed to send any other frame type */
-	WARNF("Unsupported MDP frame type: %d", mdp_type);
+	WARNF("Unsupported MDP frame type [%d] from %s", mdp_type, alloca_sockaddr(recvaddr, recvaddrlen));
 	mdp->packetTypeAndFlags=MDP_ERROR;
 	mdp->error.error=2;
 	snprintf(mdp->error.message,128,"Illegal request type.  Clients may use only MDP_TX or MDP_BIND.");

serval.h

@@ -172,7 +172,8 @@ void serval_setinstancepath(const char *instancepath);
 /* Basic socket operations.
  */
 int _esocket(struct __sourceloc, int domain, int type, int protocol);
-int _socket_setname(struct __sourceloc, struct sockaddr_un *sockname, const char *name, socklen_t *addrlen);
+int _socket_setname(struct __sourceloc, struct sockaddr_un *sockname, socklen_t *addrlen, const char *fmt, ...)
+    __attribute__((format(printf, 4, 5)));
 int _socket_bind(struct __sourceloc, int sock, const struct sockaddr *addr, socklen_t addrlen);
 int _socket_connect(struct __sourceloc, int sock, const struct sockaddr *addr, socklen_t addrlen);
 int _socket_listen(struct __sourceloc, int sock, int backlog);
@@ -180,7 +181,7 @@ int _socket_set_reuseaddr(struct __sourceloc, int sock, int reuseP);
 int _socket_set_rcvbufsize(struct __sourceloc, int sock, unsigned buffer_size);
 
 #define esocket(domain, type, protocol)             _esocket(__WHENCE__, (domain), (type), (protocol))
-#define socket_setname(sockname, name, addrlenp)    _socket_setname(__WHENCE__, (sockname), (name), (addrlenp))
+#define socket_setname(sockname, addrlenp, fmt,...) _socket_setname(__WHENCE__, (sockname), (addrlenp), (fmt), ##__VA_ARGS__)
 #define socket_bind(sock, addr, addrlen)            _socket_bind(__WHENCE__, (sock), (addr), (addrlen))
 #define socket_connect(sock, addr, addrlen)         _socket_connect(__WHENCE__, (sock), (addr), (addrlen))
 #define socket_listen(sock, backlog)                _socket_listen(__WHENCE__, (sock), (backlog))

socket.c

@@ -31,24 +31,105 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  * @author Andrew Bettison <andrew@servalproject.com>
  * @author Daniel O'Connor <daniel@servalproject.com>
  */
-int _socket_setname(struct __sourceloc __whence, struct sockaddr_un *addr, const char *name, socklen_t *addrlen)
+int _socket_setname(struct __sourceloc __whence, struct sockaddr_un *addr, socklen_t *addrlen, const char *fmt, ...)
 {
   bzero(addr, sizeof(*addr));
   addr->sun_family = AF_UNIX;
-#ifdef USE_ABSTRACT_NAMESPACE
-  addr->sun_path[0] = '\0'; // mark as Linux abstract socket
-  int len = snprintf(addr->sun_path + 1, sizeof addr->sun_path - 1, "%s.%s", DEFAULT_ABSTRACT_PREFIX, name);
-  if (len > sizeof addr->sun_path - 1)
-    return WHYF("abstract socket name overflow (%d bytes exceeds maximum %u): %s.%s", DEFAULT_ABSTRACT_PREFIX, name, len, sizeof addr->sun_path - 1);
-  *addrlen = sizeof(addr->sun_family) + 1 + len; // abstract socket names do not have a trailing nul
-#else // !USE_ABSTRACT_NAMESPACE
-  if (!FORM_SERVAL_INSTANCE_PATH(addr->sun_path, name))
-    return WHYF("local socket name overflow: %s", name);
+  va_list ap;
+  va_start(ap, fmt);
+  int r = vformf_serval_instance_path(__WHENCE__, addr->sun_path, sizeof addr->sun_path, fmt, ap);
+  va_end(ap);
+  if (r == -1)
+    return WHY("local socket name overflow");
   *addrlen = sizeof(addr->sun_family) + strlen(addr->sun_path) + 1;
-#endif // !USE_ABSTRACT_NAMESPACE
+#ifdef USE_ABSTRACT_NAMESPACE
+  // For the abstract name we use the absolute path name with the initial '/' replaced by the
+  // leading nul.  This ensures that different instances of the Serval daemon have different socket
+  // names.
+  addr->sun_path[0] = '\0'; // mark as Linux abstract socket
+  --*addrlen; // do not count trailing nul in abstract socket name
+#endif // USE_ABSTRACT_NAMESPACE
   return 0;
 }
 
+/* Compare any two struct sockaddr.  Return -1, 0 or 1.  Cope with invalid and truncated sockaddr
+ * structures.  Uses inode number comparison to resolve symbolic links in AF_UNIX path names so is
+ * not suitable for sorting, because comparison results are inconsistent (eg, if A is a symlink to
+ * C, then A == C but A < B and B < C).
+ *
+ * @author Andrew Bettison <andrew@servalproject.com>
+ */
+int cmp_sockaddr(const struct sockaddr *addrA, socklen_t addrlenA, const struct sockaddr *addrB, socklen_t addrlenB)
+{
+  // Two zero-length sockaddrs are equal.
+  if (addrlenA == 0 && addrlenB == 0)
+    return 0;
+  // If either sockaddr is truncated, then we compare the bytes we have.
+  if (addrlenA < sizeof addrA->sa_family || addrlenB < sizeof addrB->sa_family) {
+    int c = memcmp(addrA, addrB, addrlenA < addrlenB ? addrlenA : addrlenB);
+    if (c == 0)
+      c = addrlenA < addrlenB ? -1 : addrlenA > addrlenB ? 1 : 0;
+    return c;
+  }
+  // Order first by address family.
+  if (addrA->sa_family < addrB->sa_family)
+    return -1;
+  if (addrA->sa_family > addrB->sa_family)
+    return 1;
+  // Both addresses are in the same family...
+  switch (addrA->sa_family) {
+  case AF_UNIX: {
+      unsigned pathlenA = addrlenA - sizeof ((const struct sockaddr_un *)addrA)->sun_family;
+      unsigned pathlenB = addrlenB - sizeof ((const struct sockaddr_un *)addrB)->sun_family;
+      if (   pathlenA > 1 && pathlenB > 1
+	  && ((const struct sockaddr_un *)addrA)->sun_path[0] == '\0'
+	  && ((const struct sockaddr_un *)addrB)->sun_path[0] == '\0'
+      ) {
+	// Both abstract sockets - just compare names, nul bytes are not terminators.
+	int c = memcmp(&((const struct sockaddr_un *)addrA)->sun_path[1],
+		       &((const struct sockaddr_un *)addrB)->sun_path[1],
+		       (pathlenA < pathlenB ? pathlenA : pathlenB) - 1);
+	if (c == 0)
+	  c = pathlenA < pathlenB ? -1 : pathlenA > pathlenB ? 1 : 0;
+	return c;
+      }
+      // Either or both are named local file sockets.  If the file names are identical up to the
+      // first nul, then the addresses are equal.  Otherwise, if both are nul terminated file names
+      // (not abstract) then compare for equality by using the inode numbers to factor out symbolic
+      // links.  Otherwise, simply compare the nul-terminated names (abstract names start with a nul
+      // so will always collate ahead of non-empty file names).
+      int c = strncmp(((const struct sockaddr_un *)addrA)->sun_path,
+		      ((const struct sockaddr_un *)addrB)->sun_path,
+		      (pathlenA < pathlenB ? pathlenA : pathlenB));
+      if (c == 0 && pathlenA == pathlenB)
+	return 0;
+      if (   pathlenA && pathlenB
+	  && ((const struct sockaddr_un *)addrA)->sun_path[0]
+	  && ((const struct sockaddr_un *)addrB)->sun_path[0]
+	  && ((const struct sockaddr_un *)addrA)->sun_path[pathlenA - 1] == '\0'
+	  && ((const struct sockaddr_un *)addrB)->sun_path[pathlenB - 1] == '\0'
+      ) {
+	struct stat statA, statB;
+	if (   stat(((const struct sockaddr_un *)addrA)->sun_path, &statA) == 0
+	    && stat(((const struct sockaddr_un *)addrB)->sun_path, &statB) == 0
+	    && statA.st_dev == statB.st_dev
+	    && statA.st_ino == statB.st_ino
+	)
+	    return 0;
+      }
+      if (c == 0)
+	c = pathlenA < pathlenB ? -1 : pathlenA > pathlenB ? 1 : 0;
+      return c;
+    }
+    break;
+  }
+  // Fall back to comparing the data bytes.
+  int c = memcmp(addrA->sa_data, addrB->sa_data, (addrlenA < addrlenB ? addrlenA : addrlenB) - sizeof addrA->sa_family);
+  if (c == 0)
+    c = addrlenA < addrlenB ? -1 : addrlenA > addrlenB ? 1 : 0;
+  return c;
+}
+
 int _esocket(struct __sourceloc __whence, int domain, int type, int protocol)
 {
   int fd;