api: Tighter checking of structured read replies

Now that we allow clients to bypass buffer pre-initialization, it
becomes more important to detect when a buggy server using structured
replies does not send us enough bytes to cover the requested read
size.  Our check is not perfect (a server that duplicates reply chunks
for byte 0 and omits byte 1 gets past our check), but this is a
tighter sanity check so that we are less likely to report a successful
read containing uninitialized memory on a buggy server.

Because we have a maximum read buffer size of 64M, and first check
that the server's chunk fits bounds, we don't have to worry about
overflowing a uint32_t, even if a server sends enough duplicate
responses that an actual sum would overflow.

Reviewed-by: Nir Soffer <nsoffer@redhat.com>

generator/states-reply-simple.c

 /* nbd client library in userspace: state machine
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * Copyright (C) 2013-2022 Red Hat Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -40,7 +40,7 @@ STATE_MACHINE {
   if (cmd->error == 0 && cmd->type == NBD_CMD_READ) {
     h->rbuf = cmd->data;
     h->rlen = cmd->count;
-    cmd->data_seen = true;
+    cmd->data_seen = cmd->count;
     SET_NEXT_STATE (%RECV_READ_PAYLOAD);
   }
   else {

generator/states-reply-structured.c

@@ -354,7 +354,6 @@ STATE_MACHINE {
     assert (cmd); /* guaranteed by CHECK */
 
     assert (cmd->data && cmd->type == NBD_CMD_READ);
-    cmd->data_seen = true;
 
     /* Length of the data following. */
     length -= 8;
@@ -364,6 +363,8 @@ STATE_MACHINE {
       SET_NEXT_STATE (%.DEAD);
       return 0;
     }
+    if (cmd->data_seen <= cmd->count)
+      cmd->data_seen += length;
     /* Now this is the byte offset in the read buffer. */
     offset -= cmd->offset;
 
@@ -422,13 +423,14 @@ STATE_MACHINE {
     assert (cmd); /* guaranteed by CHECK */
 
     assert (cmd->data && cmd->type == NBD_CMD_READ);
-    cmd->data_seen = true;
 
     /* Is the data within bounds? */
     if (! structured_reply_in_bounds (offset, length, cmd)) {
       SET_NEXT_STATE (%.DEAD);
       return 0;
     }
+    if (cmd->data_seen <= cmd->count)
+      cmd->data_seen += length;
     /* Now this is the byte offset in the read buffer. */
     offset -= cmd->offset;
 

lib/aio.c

 /* NBD client library in userspace
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * Copyright (C) 2013-2022 Red Hat Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -91,8 +91,11 @@ nbd_unlocked_aio_command_completed (struct nbd_handle *h,
   assert (cmd->type != NBD_CMD_DISC);
   /* The spec states that a 0-length read request is unspecified; but
    * it is easy enough to treat it as successful as an extension.
+   * Conversely, make sure a server sending structured replies sent
+   * enough data chunks to cover the overall count (although we do not
+   * detect if it duplicated some bytes while omitting others).
    */
-  if (type == NBD_CMD_READ && !cmd->data_seen && cmd->count && !error)
+  if (type == NBD_CMD_READ && cmd->data_seen != cmd->count && !error)
     error = EIO;
 
   /* Retire it from the list and free it. */

lib/internal.h

@@ -352,8 +352,8 @@ struct command {
   void *data; /* Buffer for read/write */
   struct command_cb cb;
   enum state state; /* State to resume with on next POLLIN */
-  bool data_seen; /* For read, true if at least one data chunk seen */
   bool initialized; /* For read, true if getting a hole may skip memset */
+  uint32_t data_seen; /* For read, cumulative size of data chunks seen */
   uint32_t error; /* Local errno value */
 };