[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[Libguestfs] [nbdkit PATCH 1/2] connections: Report mid-message EOF as fatal



If we encounter any read error in the middle of the message
(including the NBD_CMD_WRITE payload), we are no longer in sync
with the client and should not try to read anything further.
For mid-message EOF, the problem wasn't too bad (the next
iteration of the loop would also see EOF); but if it is some
other error (perhaps a transient EIO due to reading from a
flaky block device), not marking the connection failed could
result in us trying to treat the tail of the previous partial
payload as further commands.  Usually a magic number mismatch
would flag the problem, but we should never base our behavior
on the contents of that random payload.

Signed-off-by: Eric Blake <eblake redhat com>
---
 src/connections.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/src/connections.c b/src/connections.c
index 0cbf54a..d0ef6a5 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -873,7 +873,7 @@ handle_request (struct connection *conn,
   return r;
 }

-static void
+static int
 skip_over_write_buffer (int sock, size_t count)
 {
   char buf[BUFSIZ];
@@ -883,12 +883,16 @@ skip_over_write_buffer (int sock, size_t count)
     r = read (sock, buf, count > BUFSIZ ? BUFSIZ : count);
     if (r == -1) {
       nbdkit_error ("skipping write buffer: %m");
-      return;
+      return -1;
+    }
+    if (r == 0)  {
+      nbdkit_error ("unexpected early EOF");
+      errno = EBADMSG;
+      return -1;
     }
-    if (r == 0)
-      return;
     count -= r;
   }
+  return 0;
 }

 /* Convert a system errno to an NBD_E* error code. */
@@ -965,8 +969,9 @@ recv_request_send_reply (struct connection *conn)
   if (r == -1)
     return -1;
   if (r == 0) {                 /* request not valid */
-    if (cmd == NBD_CMD_WRITE)
-      skip_over_write_buffer (conn->sockin, count);
+    if (cmd == NBD_CMD_WRITE &&
+        skip_over_write_buffer (conn->sockin, count) < 0)
+      return -1;
     goto send_reply;
   }

@@ -976,8 +981,9 @@ recv_request_send_reply (struct connection *conn)
     if (buf == NULL) {
       perror ("malloc");
       error = ENOMEM;
-      if (cmd == NBD_CMD_WRITE)
-        skip_over_write_buffer (conn->sockin, count);
+      if (cmd == NBD_CMD_WRITE &&
+          skip_over_write_buffer (conn->sockin, count) < 0)
+        return -1;
       goto send_reply;
     }
   }
@@ -985,14 +991,14 @@ recv_request_send_reply (struct connection *conn)
   /* Receive the write data buffer. */
   if (cmd == NBD_CMD_WRITE) {
     r = conn->recv (conn, buf, count);
+    if (r == 0) {
+      errno = EBADMSG;
+      r = -1;
+    }
     if (r == -1) {
       nbdkit_error ("read data: %m");
       return -1;
     }
-    if (r == 0) {
-      debug ("client closed input unexpectedly, closing connection");
-      return 0;                 /* disconnect */
-    }
   }

   /* Perform the request.  Only this part happens inside the request lock. */
-- 
2.13.6


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]