implement short read/write, EOF handling (#415)

Report any short reads to callers as ECONNRESET, which is the closest we can
meaningfully get right now. This also fixes get_next_command(), which previously
wasn't checking for short reads at all.

When we fail to send or recv from the socket due to the client disappearing in
some manner, call into vfu_reset_ctx() to clean up the connection fd, allowing a
subsequent vfu_attach_ctx() to work.

If we get 0 bytes from recv[msg](), this is reported by the transport as ENOMSG,
and is a normal EOF condition.

We can also get ECONNRESET: this can happen when we've written unacknowledged
data to the socket, the client side socket is closed, and we try a subsequent
read.

Finally, we can get a short read or write. Our handling of these still has
issues, but for now we'll presume this means the client has gone too. It may
in fact be due to a client bug - if it failed to write enough data - but right
now, we can't easily tell that.

Signed-off-by: John Levon <john.levon@nutanix.com>
Reviewed-by: Thanos Makatos <thanos.makatos@nutanix.com>

3 files changed, 84 insertions, 31 deletions

diff --git a/include/libvfio-user.h b/include/libvfio-user.h
index 1048236..925550a 100644
--- a/include/libvfio-user.h
+++ b/include/libvfio-user.h
@@ -137,18 +137,22 @@ int
 vfu_get_poll_fd(vfu_ctx_t *vfu_ctx);
 
 /**
- * Polls the vfu_ctx and processes the command recieved from client.
+ * Polls the vfu_ctx and processes the command received from client.
  * - Blocking vfu_ctx:
  *   Blocks until new request is received from client and continues processing
  *   the requests. Exits only in case of error or if the client disconnects.
  * - Non-blocking vfu_ctx(LIBVFIO_USER_FLAG_ATTACH_NB):
  *   Processes one request from client if it's available, otherwise it
- *   immediatelly returns and the caller is responsible for periodically
+ *   immediately returns and the caller is responsible for periodically
  *   calling again.
  *
  * @vfu_ctx: The libvfio-user context to poll
  *
- * @returns 0 on success, -1 on error. Sets errno.
+ * @returns 0 on success, -1 on error, with errno set as follows:
+ *
+ * EAGAIN/EWOULDBLOCK: no more commands to process
+ * ENOTCONN: client closed connection, vfu_attach_ctx() should be called again
+ * Other errno values are also possible.
  */
 int
 vfu_run_ctx(vfu_ctx_t *vfu_ctx);
diff --git a/lib/libvfio-user.c b/lib/libvfio-user.c
index ebd70af..c041694 100644
--- a/lib/libvfio-user.c
+++ b/lib/libvfio-user.c
@@ -56,6 +56,7 @@
 #include "private.h"
 #include "tran_sock.h"
 
+static void vfu_reset_ctx(vfu_ctx_t *vfu_ctx, const char *reason);
 
 void
 vfu_log(vfu_ctx_t *vfu_ctx, int level, const char *fmt, ...)
@@ -698,26 +699,25 @@ MOCK_DEFINE(get_next_command)(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr,
     /* FIXME get request shouldn't set errno, it should return it as -errno */
     ret = vfu_ctx->tran->get_request(vfu_ctx, hdr, fds, nr_fds);
     if (unlikely(ret < 0)) {
-        if (ret == -EAGAIN || ret == -EWOULDBLOCK) {
+        switch (-ret) {
+        case EAGAIN:
             return 0;
-        }
-        if (ret != -EINTR) {
+
+        case ENOMSG:
+            vfu_reset_ctx(vfu_ctx, "closed");
+            return -ENOTCONN;
+
+        case ECONNRESET:
+            vfu_reset_ctx(vfu_ctx, "reset");
+            return -ENOTCONN;
+
+        default:
             vfu_log(vfu_ctx, LOG_ERR, "failed to receive request: %s",
                    strerror(-ret));
+            return ret;
         }
-        return ret;
-    }
-    if (unlikely(ret == 0)) {
-        if (errno == EINTR) {
-            return -EINTR;
-        }
-        if (errno == 0) {
-            vfu_log(vfu_ctx, LOG_INFO, "vfio-user client closed connection");
-        } else {
-            vfu_log(vfu_ctx, LOG_ERR, "end of file: %m");
-        }
-        return -ENOTCONN;
     }
+
     return ret;
 }
 
@@ -784,7 +784,13 @@ MOCK_DEFINE(exec_command)(vfu_ctx_t *vfu_ctx, struct vfio_user_header *hdr,
     if (cmd_data_size > 0) {
         ret = vfu_ctx->tran->recv_body(vfu_ctx, hdr, &cmd_data);
 
-        if (ret < 0) {
+        if (ret == -ENOMSG) {
+            vfu_reset_ctx(vfu_ctx, "closed");
+            return -ENOTCONN;
+        } else if (ret == -ECONNRESET) {
+            vfu_reset_ctx(vfu_ctx, "reset");
+            return -ENOTCONN;
+        } else if (ret < 0) {
             return ret;
         }
     }
@@ -943,6 +949,10 @@ MOCK_DEFINE(process_request)(vfu_ctx_t *vfu_ctx)
     if (ret < 0) {
         vfu_log(vfu_ctx, LOG_ERR, "msg%#hx: cmd %d failed: %s", hdr.msg_id,
                 hdr.cmd, strerror(-ret));
+
+        if (ret == -ENOTCONN) {
+            goto out;
+        }
     } else {
         ret = 0;
     }
@@ -955,11 +965,21 @@ MOCK_DEFINE(process_request)(vfu_ctx_t *vfu_ctx)
     } else {
         ret = vfu_ctx->tran->reply(vfu_ctx, hdr.msg_id, iovecs, nr_iovecs,
                                    fds_out, nr_fds_out, -ret);
-        if (unlikely(ret < 0)) {
+
+        if (ret < 0) {
             vfu_log(vfu_ctx, LOG_ERR, "failed to reply: %s", strerror(-ret));
+
+            if (ret == -ECONNRESET) {
+                vfu_reset_ctx(vfu_ctx, "reset");
+                ret = -ENOTCONN;
+            } else if (ret == -ENOMSG) {
+                vfu_reset_ctx(vfu_ctx, "closed");
+                ret = -ENOTCONN;
+            }
         }
     }
 
+out:
     if (iovecs != NULL) {
         if (free_iovec_data) {
             size_t i;
@@ -1087,6 +1107,16 @@ free_sparse_mmap_areas(vfu_ctx_t *vfu_ctx)
     }
 }
 
+static void
+vfu_reset_ctx(vfu_ctx_t *vfu_ctx, const char *reason)
+{
+    vfu_log(vfu_ctx, LOG_INFO, "%s: %s", __func__,  reason);
+
+    if (vfu_ctx->tran->detach != NULL) {
+        vfu_ctx->tran->detach(vfu_ctx);
+    }
+}
+
 void
 vfu_destroy_ctx(vfu_ctx_t *vfu_ctx)
 {
@@ -1095,11 +1125,10 @@ vfu_destroy_ctx(vfu_ctx_t *vfu_ctx)
         return;
     }
 
+    vfu_reset_ctx(vfu_ctx, "destroyed");
+
     free(vfu_ctx->uuid);
     free(vfu_ctx->pci.config_space);
-    if (vfu_ctx->tran->detach != NULL) {
-        vfu_ctx->tran->detach(vfu_ctx);
-    }
 
     if (vfu_ctx->tran->fini != NULL) {
         vfu_ctx->tran->fini(vfu_ctx);
@@ -1519,7 +1548,18 @@ vfu_dma_read(vfu_ctx_t *vfu_ctx, dma_sg_t *sg, void *data)
     ret = vfu_ctx->tran->send_msg(vfu_ctx, msg_id, VFIO_USER_DMA_READ,
                                   &dma_send, sizeof(dma_send), NULL,
                                   dma_recv, recv_size);
-    memcpy(data, dma_recv->data, sg->length); /* FIXME no need for memcpy */
+
+    if (ret == -ENOMSG) {
+        vfu_reset_ctx(vfu_ctx, "closed");
+        ret = -ENOTCONN;
+    } else if (ret == -ECONNRESET) {
+        vfu_reset_ctx(vfu_ctx, "reset");
+        ret = -ENOTCONN;
+    } else if (ret == 0) {
+        /* FIXME no need for memcpy */
+        memcpy(data, dma_recv->data, sg->length);
+    }
+
     free(dma_recv);
 
     return ret < 0 ? ERROR_INT(-ret) : 0;
@@ -1545,6 +1585,15 @@ vfu_dma_write(vfu_ctx_t *vfu_ctx, dma_sg_t *sg, void *data)
     ret = vfu_ctx->tran->send_msg(vfu_ctx, msg_id, VFIO_USER_DMA_WRITE,
                                   dma_send, send_size, NULL,
                                   &dma_recv, sizeof(dma_recv));
+
+    if (ret == -ENOMSG) {
+        vfu_reset_ctx(vfu_ctx, "closed");
+        ret = -ENOTCONN;
+    } else if (ret == -ECONNRESET) {
+        vfu_reset_ctx(vfu_ctx, "reset");
+        ret = -ENOTCONN;
+    }
+
     free(dma_send);
 
     return ret < 0 ? ERROR_INT(-ret) : 0;
diff --git a/lib/tran_sock.c b/lib/tran_sock.c
index 9a58570..ea01a68 100644
--- a/lib/tran_sock.c
+++ b/lib/tran_sock.c
@@ -173,6 +173,8 @@ get_msg(void *data, size_t len, int *fds, size_t *nr_fds, int sock_fd,
         return -errno;
     } else if (ret == 0) {
         return -ENOMSG;
+    } else if ((size_t)ret < len) {
+        return -ECONNRESET;
     }
 
     if (msg.msg_flags & MSG_CTRUNC || msg.msg_flags & MSG_TRUNC) {
@@ -222,9 +224,6 @@ tran_sock_recv_fds(int sock, struct vfio_user_header *hdr, bool is_reply,
     if (ret < 0) {
         return ret;
     }
-    if (ret < (int)sizeof(*hdr)) {
-        return -EINVAL;
-    }
 
     if (is_reply) {
         if (msg_id != NULL && hdr->msg_id != *msg_id) {
@@ -257,11 +256,12 @@ tran_sock_recv_fds(int sock, struct vfio_user_header *hdr, bool is_reply,
             return -errno;
         } else if (ret == 0) {
             return -ENOMSG;
-        } else if (*len != (size_t)ret) { /* FIXME we should allow receiving less */
-            return -EINVAL;
+        } else if (*len != (size_t)ret) {
+            return -ECONNRESET;
         }
         *len = ret;
     }
+
     return 0;
 }
 
@@ -318,7 +318,7 @@ tran_sock_recv_alloc(int sock, struct vfio_user_header *hdr, bool is_reply,
         return -ENOMSG;
     } else if (len != (size_t)ret) {
         free(data);
-        return -EINVAL;
+        return -ECONNRESET;
     }
 
     *datap = data;
@@ -818,7 +818,7 @@ tran_sock_recv_body(vfu_ctx_t *vfu_ctx, const struct vfio_user_header *hdr,
         vfu_log(vfu_ctx, LOG_ERR, "msg%#hx: short read: expected=%d, actual=%d",
                 hdr->msg_id, body_size, ret);
         free(data);
-        return -EINVAL;
+        return -ECONNRESET;
     }
 
     *datap = data;