Fix MDP client packet receive bug on OSX

Was failing because recvmsg(2) on OSX does not nul terminate the
pathname of local (AF_UNIX) sockets in the returned sockaddr buffer.
Zerofilling the buffer before calling recvmsg() does the trick.

In the process, improved debug and error logging, replacing
recvwithttl() with recv_message() and recv_message_frag().  The
mdp_net.c source file was retired.

mdp_client.c

@@ -323,12 +323,8 @@ int overlay_mdp_recv(int mdp_sockfd, overlay_mdp_frame *mdp, mdp_port_t port, in
   ssize_t len;
   mdp->packetTypeAndFlags = 0;
   set_nonblock(mdp_sockfd);
-  len = recvwithttl(mdp_sockfd, (unsigned char *)mdp, sizeof(overlay_mdp_frame), ttl, &recvaddr);
-  if (len == -1)
-    WHYF_perror("recvwithttl(%d,%p,%zu,&%d,%p(%s))",
-	  mdp_sockfd, mdp, sizeof(overlay_mdp_frame), *ttl,
-	  &recvaddr, alloca_socket_address(&recvaddr)
-	);
+
+  len = recv_message(mdp_sockfd, &recvaddr, ttl, (unsigned char *)mdp, sizeof(overlay_mdp_frame));
   set_block(mdp_sockfd);
   if (len <= 0)
     return -1; // no packet received
@@ -336,7 +332,7 @@ int overlay_mdp_recv(int mdp_sockfd, overlay_mdp_frame *mdp, mdp_port_t port, in
   // If the received address overflowed the buffer, then it cannot have come from the server, whose
   // address must always fit within a struct sockaddr_un.
   if (recvaddr.addrlen > sizeof recvaddr.store)
-    return WHY("reply did not come from server: address overrun");
+    return WHYF("reply did not come from server %s: address overrun", alloca_socket_address(&mdp_addr));
 
   // Compare the address of the sender with the address of our server, to ensure they are the same.
   // If the comparison fails, then try using realpath(3) on the sender address and compare again.
@@ -346,7 +342,7 @@ int overlay_mdp_recv(int mdp_sockfd, overlay_mdp_frame *mdp, mdp_port_t port, in
 	  || cmp_sockaddr(&recvaddr, &mdp_addr) != 0
 	 )
   )
-    return WHYF("reply did not come from server: %s", alloca_socket_address(&recvaddr));
+    return WHYF("reply did not come from server %s: %s", alloca_socket_address(&mdp_addr), alloca_socket_address(&recvaddr));
   
   // silently drop incoming packets for the wrong port number
   if (port>0 && port != mdp->out.dst.port){

mdp_net.c

@@ -1,70 +0,0 @@
-/*
-Copyright (C) 2013 Serval Project Inc.
- 
-This program is free software; you can redistribute it and/or
-modify it under the terms of the GNU General Public License
-as published by the Free Software Foundation; either version 2
-of the License, or (at your option) any later version.
- 
-This program is distributed in the hope that it will be useful,
-but WITHOUT ANY WARRANTY; without even the implied warranty of
-MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-GNU General Public License for more details.
- 
-You should have received a copy of the GNU General Public License
-along with this program; if not, write to the Free Software
-Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
-*/
-
-#include "socket.h"
-#include "conf.h"
-#include "log.h"
-#include "debug.h"
-
-ssize_t recvwithttl(int sock,unsigned char *buffer, size_t bufferlen,int *ttl, struct socket_address *recvaddr)
-{
-  struct msghdr msg;
-  struct iovec iov[1];
-  struct cmsghdr cmsgcmsg[16];
-  iov[0].iov_base=buffer;
-  iov[0].iov_len=bufferlen;
-  bzero(&msg,sizeof(msg));
-  msg.msg_name = &recvaddr->store;
-  msg.msg_namelen = sizeof(recvaddr->store);
-  msg.msg_iov = &iov[0];
-  msg.msg_iovlen = 1;
-  msg.msg_control = cmsgcmsg;
-  msg.msg_controllen = sizeof cmsgcmsg;
-  msg.msg_flags = 0;
-  
-  ssize_t len = recvmsg(sock,&msg,0);
-  if (len == -1 && errno != EAGAIN && errno != EWOULDBLOCK)
-    return WHYF_perror("recvmsg(%d,%p,0)", sock, &msg);
-  
-#if 0
-  DEBUGF(packetrx, "recvmsg returned %d (flags=%d, msg_controllen=%d)", (int) len, msg.msg_flags, (int)msg.msg_controllen);
-  if (IF_DEBUG(packetrx))
-    dump("received data", buffer, len);
-#endif
-  
-  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
-      ) {
-	DEBUGF(packetrx, "  TTL (%p) data location resolves to %p", ttl,CMSG_DATA(cmsg));
-	if (CMSG_DATA(cmsg)) {
-	  *ttl = *(unsigned char *) CMSG_DATA(cmsg);
-	  DEBUGF(packetrx, "  TTL of packet is %d", *ttl);
-	} 
-      } else {
-	DEBUGF(packetrx, "I didn't expect to see level=%02x, type=%02x", cmsg->cmsg_level,cmsg->cmsg_type);
-      }	 
-    }
-  }
-  recvaddr->addrlen = msg.msg_namelen;
-  
-  return len;
-}

overlay_interface.c

@@ -380,12 +380,8 @@ overlay_interface_read_any(struct sched_ent *alarm)
     
     /* Read only one UDP packet per call to share resources more fairly, and also
      enable stats to accurately count packets received */
-    plen = recvwithttl(alarm->poll.fd, packet, sizeof(packet), &recvttl, &recvaddr);
+    plen = recv_message(alarm->poll.fd, &recvaddr, &recvttl, packet, sizeof(packet));
     if (plen == -1) {
-      WHYF_perror("recvwithttl(%d,%p,%zu,&%d,%p(%s))",
-	    alarm->poll.fd, packet, sizeof packet, recvttl,
-	    &recvaddr, alloca_socket_address(&recvaddr)
-	  );
       unwatch(alarm);
       close(alarm->poll.fd);
       return;
@@ -683,12 +679,8 @@ static void interface_read_dgram(struct overlay_interface *interface)
   /* Read only one UDP packet per call to share resources more fairly, and also
    enable stats to accurately count packets received */
   int recvttl=1;
-  plen = recvwithttl(interface->alarm.poll.fd,packet, sizeof(packet), &recvttl, &recvaddr);
+  plen = recv_message(interface->alarm.poll.fd, &recvaddr, &recvttl, packet, sizeof(packet));
   if (plen == -1) {
-    WHYF_perror("recvwithttl(%d,%p,%zu,&%d,%p(%s))",
-	  interface->alarm.poll.fd, packet, sizeof packet, recvttl,
-	  &recvaddr, alloca_socket_address(&recvaddr)
-	);
     overlay_interface_close(interface);
     return;
   }

overlay_mdp.c

@@ -1717,18 +1717,10 @@ static void overlay_mdp_poll(struct sched_ent *alarm)
 {
   if (alarm->poll.revents & POLLIN) {
     unsigned char buffer[16384];
-    int ttl;
+    int ttl = -1;
     struct socket_address client;
     client.addrlen = sizeof client.store;
-
-    ttl=-1;
-    
-    ssize_t len = recvwithttl(alarm->poll.fd,buffer,sizeof(buffer),&ttl, &client);
-    if (len == -1)
-      WHYF_perror("recvwithttl(%d,%p,%zu,&%d,%p(%s))",
-	    alarm->poll.fd, buffer, sizeof buffer, ttl,
-	    &client, alloca_socket_address(&client)
-	  );
+    ssize_t len = recv_message(alarm->poll.fd, &client, &ttl, buffer, sizeof(buffer));
 
     if ((size_t)len > 0) {
       if (client.addrlen <= sizeof(sa_family_t))

socket.c

@@ -84,6 +84,7 @@ int _make_local_sockaddr(struct __sourceloc __whence, struct socket_address *add
  */
 int real_sockaddr(const struct socket_address *src_addr, struct socket_address *dst_addr)
 {
+  DEBUGF2(io, verbose_io, "real_sockaddr(src_addr=%p %s, dst_addr=%p)", src_addr, alloca_socket_address(src_addr), dst_addr);
   assert(src_addr->addrlen > sizeof src_addr->local.sun_family);
   size_t src_path_len = src_addr->addrlen - sizeof src_addr->local.sun_family;
   if (	 src_addr->addrlen >= sizeof src_addr->local.sun_family + 1
@@ -103,8 +104,10 @@ int real_sockaddr(const struct socket_address *src_addr, struct socket_address *
     ) {
       memcpy(dst_addr->local.sun_path, real_path, real_path_len);
       dst_addr->addrlen = real_path_len + sizeof dst_addr->local.sun_family;
+      DEBUGF2(io, verbose_io, "   --> return %s", alloca_socket_address(dst_addr));
       return 1;
     }
+    DEBUGF2(io, verbose_io, "real_path=%s", alloca_str_toprint(real_path));
   }
   if (dst_addr != src_addr){
     memcpy(&dst_addr->addr, &src_addr->addr, src_addr->addrlen);
@@ -280,20 +283,61 @@ ssize_t _send_message(struct __sourceloc __whence, int fd, const struct socket_a
   ssize_t ret = sendmsg(fd, &hdr, 0);
   if (ret == -1 && errno != EAGAIN)
     WHYF_perror("sendmsg(%d,%s,%lu)", fd, alloca_socket_address(address), (unsigned long)address->addrlen);
+  DEBUGF(verbose_io, "sendmsg(%d, %s, %lu)", fd, alloca_socket_address(address), (unsigned long)address->addrlen);
   return ret;
 }
 
-ssize_t _recv_message(struct __sourceloc __whence, int fd, struct socket_address *address, struct fragmented_data *data)
+ssize_t _recv_message_frag(struct __sourceloc __whence, int fd, struct socket_address *address, int *ttl, struct fragmented_data *data)
 {
-  struct msghdr hdr={
-    .msg_name=(void *)&address->addr,
-    .msg_namelen=address->addrlen,
-    .msg_iov=data->iov,
-    .msg_iovlen=data->fragment_count,
+  struct cmsghdr cmsgs[16];
+  struct msghdr msg = {
+    .msg_name       = (void *)&address->addr,
+    .msg_namelen    = address->addrlen,
+    .msg_iov        = data->iov,
+    .msg_iovlen     = data->fragment_count,
+    .msg_control    = cmsgs,
+    .msg_controllen = sizeof cmsgs,
+    .msg_flags      = 0
   };
-  ssize_t ret = recvmsg(fd, &hdr, 0);
-  if (ret==-1)
-    WHYF_perror("recvmsg(%d,%s,%lu)", fd, alloca_socket_address(address), (unsigned long)address->addrlen);
-  address->addrlen = hdr.msg_namelen;
+  bzero(&address->addr, address->addrlen);
+  ssize_t ret = recvmsg(fd, &msg, 0);
+  if (ret == -1 && errno != EAGAIN && errno != EWOULDBLOCK)
+    WHYF_perror("recvmsg(%d,{name=%p,namelen=%u,iov=%s,control=%p,controllen=%u},0)",
+	  fd, &address->addr, (unsigned) address->addrlen,
+          alloca_iovec(data->iov, data->fragment_count),
+          cmsgs, (unsigned) sizeof cmsgs);
+  address->addrlen = msg.msg_namelen;
+  if (ttl && ret > 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
+      ) {
+	DEBUGF(verbose_io, "  TTL (%p) data location resolves to %p", ttl, CMSG_DATA(cmsg));
+	if (CMSG_DATA(cmsg)) {
+	  *ttl = *(unsigned char *) CMSG_DATA(cmsg);
+	  DEBUGF(verbose_io, "  TTL of packet is %d", *ttl);
+	}
+      } else {
+	DEBUGF(verbose_io, "  unexpected level=%02x, type=%02x", cmsg->cmsg_level, cmsg->cmsg_type);
+      }
+    }
+  }
+  DEBUGF(verbose_io, "recvmsg(%d) -> %zd, flags=%x, address=%s ttl=%d",
+         fd,
+         ret,
+         msg.msg_flags,
+         alloca_socket_address(address),
+         ttl ? *ttl : -1);
   return ret;
 }
+
+ssize_t _recv_message(struct __sourceloc __whence, int fd, struct socket_address *address, int *ttl, unsigned char *buffer, size_t buflen)
+{
+  struct fragmented_data data;
+  data.fragment_count = 1;
+  data.iov[0].iov_base = buffer;
+  data.iov[0].iov_len = buflen;
+  return _recv_message_frag(__whence, fd, address, ttl, &data);
+}

socket.h

@@ -81,11 +81,11 @@ int append_fragment(struct fragmented_data *data, const uint8_t *payload, size_t
 size_t copy_fragment(struct fragmented_data *src, uint8_t *dest, size_t length);
 
 ssize_t _send_message(struct __sourceloc, int fd, const struct socket_address *address, const struct fragmented_data *data);
-ssize_t _recv_message(struct __sourceloc, int fd, struct socket_address *address, struct fragmented_data *data);
+ssize_t _recv_message_frag(struct __sourceloc, int fd, struct socket_address *address, int *ttl, struct fragmented_data *data);
+ssize_t _recv_message(struct __sourceloc __whence, int fd, struct socket_address *address, int *ttl, unsigned char *buffer, size_t buflen);
 
 #define send_message(fd, address, data)      _send_message(__WHENCE__, (fd), (address), (data))
-#define recv_message(fd, address, data)    _recv_message(__WHENCE__, (fd), (address), (data))
-
-ssize_t recvwithttl(int sock, unsigned char *buffer, size_t bufferlen, int *ttl, struct socket_address *recvaddr);
+#define recv_message_frag(fd, address, ttl, data) _recv_message(__WHENCE__, (fd), (address), (ttl), (data))
+#define recv_message(fd, address, ttl, buf, len) _recv_message(__WHENCE__, (fd), (address), (ttl), (buf), (len))
 
 #endif // __SERVAL_DNA___SOCKET_H

sourcefiles.mk

@@ -115,8 +115,7 @@ TEST_SOURCES = \
 	context1.c
 
 MDP_CLIENT_SOURCES = \
-	mdp_client.c \
-        mdp_net.c
+	mdp_client.c
 
 SIMULATOR_SOURCES = \
         simulator.c