- Bugfixes in unixmessage/skaclient (short writes / fd leakage / DoS)v2.1.0.0

 - ABI change: unixmessage protocol header is now 6 bytes (was 8)
 - API change: skaclient_start(_async) now takes an "options" argument
 - version increase to 2.1.0.0

1 files changed, 48 insertions, 17 deletions

diff --git a/src/libunixonacid/unixmessage_sender_flush.c b/src/libunixonacid/unixmessage_sender_flush.c
index bd4c704..bea3585 100644
--- a/src/libunixonacid/unixmessage_sender_flush.c
+++ b/src/libunixonacid/unixmessage_sender_flush.c
@@ -6,9 +6,10 @@
 #include <sys/uio.h>
 #include <unistd.h>
 #include <errno.h>
-#include <skalibs/uint.h>
+#include <skalibs/uint16.h>
+#include <skalibs/uint32.h>
 #include <skalibs/diuint.h>
-#include <skalibs/stralloc.h>
+#include <skalibs/allreadwrite.h>
 #include <skalibs/genalloc.h>
 #include <skalibs/djbunix.h>
 #include <skalibs/unixmessage.h>
@@ -18,21 +19,51 @@
 #define MSG_NOSIGNAL 0
 #endif
 
+ /*
+    XXX: sendmsg/recvmsg is badly, badly specified.
+    XXX: We assume ancillary data is attached to the first byte.
+ */
+
 int unixmessage_sender_flush (unixmessage_sender_t *b)
 {
-  diuint last = { .left = b->data.len, .right = genalloc_len(int, &b->fds) } ;
+  diuint last = { .left = (uint32)b->data.len, .right = genalloc_len(int, &b->fds) } ;
   diuint *offsets = genalloc_s(diuint, &b->offsets) ;
   unsigned int n = genalloc_len(diuint, &b->offsets) ;
-  unsigned int oldhead = b->head ;
+  register int r ;
+
+  if (b->shorty) /* we had a short write, gotta send the remainder first */
+  {
+    diuint *next = b->head+1 < n ? offsets + b->head+1 : &last ;
+    unsigned int len = next->left - offsets[b->head].left ;
+    if (b->shorty <= len)
+      r = fd_write(b->fd, b->data.s + offsets[b->head].left + (len - b->shorty), b->shorty) ;
+    else
+    {
+      unsigned int nfds = next->right - offsets[b->head].right ;
+      char pack[6] ;
+      struct iovec v[2] =
+      {
+        { .iov_base = pack + 6 - (b->shorty - len), .iov_len = b->shorty - len },
+        { .iov_base = b->data.s + offsets[b->head].left, .iov_len = len }
+      } ;
+      uint32_pack_big(pack, (uint32)len) ;
+      uint16_pack_big(pack + 4, (uint16)nfds) ;
+      r = fd_writev(b->fd, v, 2) ;
+    }
+    if (r <= 0) return 0 ;
+    b->shorty -= r ;
+    if (b->shorty) return (errno = EWOULDBLOCK, 0) ;
+  }
+
   for (; b->head < n ; b->head++)
   {
     diuint *next = b->head+1 < n ? offsets + b->head+1 : &last ;
     unsigned int len = next->left - offsets[b->head].left ;
     unsigned int nfds = next->right - offsets[b->head].right ;
-    char pack[sizeof(unsigned int) << 1] ;
+    char pack[6] ;
     struct iovec v[2] =
     {
-      { .iov_base = pack, .iov_len = sizeof(unsigned int) << 1 },
+      { .iov_base = pack, .iov_len = 6 },
       { .iov_base = b->data.s + offsets[b->head].left, .iov_len = len }
     } ;
     char ancilbuf[CMSG_SPACE(nfds * sizeof(int))] ;
@@ -45,8 +76,8 @@ int unixmessage_sender_flush (unixmessage_sender_t *b)
       .msg_control = nfds ? ancilbuf : 0,
       .msg_controllen = nfds ? sizeof(ancilbuf) : 0
     } ;
-    uint_pack_big(pack, len) ;
-    uint_pack_big(pack + sizeof(unsigned int), nfds) ;
+    uint32_pack_big(pack, (uint32)len) ;
+    uint16_pack_big(pack + 4, (uint16)nfds) ;
     if (nfds)
     {
       struct cmsghdr *cp = CMSG_FIRSTHDR(&hdr) ;
@@ -60,14 +91,9 @@ int unixmessage_sender_flush (unixmessage_sender_t *b)
         ((int *)CMSG_DATA(cp))[i] = fd < 0 ? -(fd+1) : fd ;
       }
     }
-    for (;;)
-    {
-      register int r = sendmsg(b->fd, &hdr, MSG_NOSIGNAL) ;
-      if (r == -1 && errno == EINTR) continue ;
-      if (r < (int)(len + (sizeof(unsigned int) << 1)))
-        return -(int)(b->head-oldhead)-1 ;
-      break ;
-    }
+    do r = sendmsg(b->fd, &hdr, MSG_NOSIGNAL) ;
+    while (r < 0 && errno == EINTR) ;
+    if (r <= 0) return 0 ;
 #ifndef SKALIBS_HASANCILAUTOCLOSE
     if (nfds)
     {
@@ -79,10 +105,15 @@ int unixmessage_sender_flush (unixmessage_sender_t *b)
       }
     }
 #endif
+    if ((unsigned int)r < 6 + len)
+    {
+      b->shorty = 6 + len - r ;
+      return (errno = EWOULDBLOCK, 0) ;
+    }
   }
   b->data.len = 0 ;
   genalloc_setlen(int, &b->fds, 0) ;
   genalloc_setlen(diuint, &b->offsets, 0) ;
   b->head = 0 ;
-  return (int)(n - oldhead) ;
+  return 1 ;
 }