diff mbox series

[libnbd,05/13] protocol: Prepare to receive 64-bit replies

Message ID 20211203231741.3901263-6-eblake@redhat.com (mailing list archive)
State New, archived
Headers show
Series spec: Add NBD_OPT_EXTENDED_HEADERS | expand

Commit Message

Eric Blake Dec. 3, 2021, 11:17 p.m. UTC 
Support receiving headers for 64-bit replies if extended headers were
negotiated.  We already insist that the server not send us too much
payload in one reply, so we can exploit that and merge the 64-bit
length back into a normalized 32-bit field for the rest of the payload
length calculations.  The NBD protocol specifically made extended
simple and structured replies both occupy 32 bytes, while the handle
field is still in the same offset between all reply types.

Note that if we negotiate extended headers, but a non-compliant server
replies with a non-extended header, we will stall waiting for the
server to send more bytes rather than noticing that the magic number
is wrong.  The alternative would be to read just the first 4 bytes of
magic, then determine how many more bytes to expect; but that would
require more states and syscalls, and not worth it since the typical
server will be compliant.

At this point, h->extended_headers is permanently false (we can't
enable it until all other aspects of the protocol have likewise been
converted).
---
 lib/internal.h                      |  8 +++-
 generator/states-reply-structured.c | 59 +++++++++++++++++++----------
 generator/states-reply.c            | 31 +++++++++++----
 3 files changed, 68 insertions(+), 30 deletions(-)
diff mbox series

Patch

diff --git a/lib/internal.h b/lib/internal.h
index 07378588..c9f84441 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -222,8 +222,12 @@  struct nbd_handle {
     }  __attribute__((packed)) or;
     struct nbd_export_name_option_reply export_name_reply;
     struct nbd_simple_reply simple_reply;
+    struct nbd_simple_reply_ext simple_reply_ext;
     struct {
-      struct nbd_structured_reply structured_reply;
+      union {
+        struct nbd_structured_reply structured_reply;
+        struct nbd_structured_reply_ext structured_reply_ext;
+      } hdr;
       union {
         struct nbd_structured_reply_offset_data offset_data;
         struct nbd_structured_reply_offset_hole offset_hole;
@@ -233,7 +237,7 @@  struct nbd_handle {
           uint64_t offset; /* Only used for NBD_REPLY_TYPE_ERROR_OFFSET */
         } __attribute__((packed)) error;
       } payload;
-    }  __attribute__((packed)) sr;
+    } sr;
     uint16_t gflags;
     uint32_t cflags;
     uint32_t len;
diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c
index 5524e000..1b675e8d 100644
--- a/generator/states-reply-structured.c
+++ b/generator/states-reply-structured.c
@@ -45,19 +45,23 @@  structured_reply_in_bounds (uint64_t offset, uint32_t length,

 STATE_MACHINE {
  REPLY.STRUCTURED_REPLY.START:
-  /* We've only read the simple_reply.  The structured_reply is longer,
-   * so read the remaining part.
+  /* We've only read the simple_reply.  Unless we have extended headers,
+   * the structured_reply is longer, so read the remaining part.
    */
   if (!h->structured_replies) {
     set_error (0, "server sent unexpected structured reply");
     SET_NEXT_STATE(%.DEAD);
     return 0;
   }
-  h->rbuf = &h->sbuf;
-  h->rbuf += sizeof h->sbuf.simple_reply;
-  h->rlen = sizeof h->sbuf.sr.structured_reply;
-  h->rlen -= sizeof h->sbuf.simple_reply;
-  SET_NEXT_STATE (%RECV_REMAINING);
+  if (h->extended_headers)
+    SET_NEXT_STATE (%CHECK);
+  else {
+    h->rbuf = &h->sbuf;
+    h->rbuf += sizeof h->sbuf.simple_reply;
+    h->rlen = sizeof h->sbuf.sr.hdr.structured_reply;
+    h->rlen -= sizeof h->sbuf.simple_reply;
+    SET_NEXT_STATE (%RECV_REMAINING);
+  }
   return 0;

  REPLY.STRUCTURED_REPLY.RECV_REMAINING:
@@ -75,12 +79,21 @@  STATE_MACHINE {
   struct command *cmd = h->reply_cmd;
   uint16_t flags, type;
   uint64_t cookie;
-  uint32_t length;
+  uint64_t length;

-  flags = be16toh (h->sbuf.sr.structured_reply.flags);
-  type = be16toh (h->sbuf.sr.structured_reply.type);
-  cookie = be64toh (h->sbuf.sr.structured_reply.handle);
-  length = be32toh (h->sbuf.sr.structured_reply.length);
+  flags = be16toh (h->sbuf.sr.hdr.structured_reply.flags);
+  type = be16toh (h->sbuf.sr.hdr.structured_reply.type);
+  cookie = be64toh (h->sbuf.sr.hdr.structured_reply.handle);
+  if (h->extended_headers) {
+    length = be64toh (h->sbuf.sr.hdr.structured_reply_ext.length);
+    if (h->sbuf.sr.hdr.structured_reply_ext.pad) {
+      set_error (0, "server sent non-zero padding in structured reply header");
+      SET_NEXT_STATE(%.DEAD);
+      return 0;
+    }
+  }
+  else
+    length = be32toh (h->sbuf.sr.hdr.structured_reply.length);

   assert (cmd);
   assert (cmd->cookie == cookie);
@@ -97,6 +110,10 @@  STATE_MACHINE {
     SET_NEXT_STATE (%.DEAD);
     return 0;
   }
+  /* For convenience, we now normalize extended replies into compact,
+   * doable since we validated length fits in 32 bits.
+   */
+  h->sbuf.sr.hdr.structured_reply.length = length;

   if (NBD_REPLY_TYPE_IS_ERR (type)) {
     if (length < sizeof h->sbuf.sr.payload.error.error) {
@@ -207,7 +224,7 @@  STATE_MACHINE {
     SET_NEXT_STATE (%.READY);
     return 0;
   case 0:
-    length = be32toh (h->sbuf.sr.structured_reply.length);
+    length = h->sbuf.sr.hdr.structured_reply.length; /* normalized in CHECK */
     msglen = be16toh (h->sbuf.sr.payload.error.error.len);
     if (msglen > length - sizeof h->sbuf.sr.payload.error.error ||
         msglen > sizeof h->sbuf.sr.payload.error.msg) {
@@ -233,9 +250,9 @@  STATE_MACHINE {
     SET_NEXT_STATE (%.READY);
     return 0;
   case 0:
-    length = be32toh (h->sbuf.sr.structured_reply.length);
+    length = h->sbuf.sr.hdr.structured_reply.length; /* normalized in CHECK */
     msglen = be16toh (h->sbuf.sr.payload.error.error.len);
-    type = be16toh (h->sbuf.sr.structured_reply.type);
+    type = be16toh (h->sbuf.sr.hdr.structured_reply.type);

     length -= sizeof h->sbuf.sr.payload.error.error + msglen;

@@ -281,7 +298,7 @@  STATE_MACHINE {
     return 0;
   case 0:
     error = be32toh (h->sbuf.sr.payload.error.error.error);
-    type = be16toh (h->sbuf.sr.structured_reply.type);
+    type = be16toh (h->sbuf.sr.hdr.structured_reply.type);

     assert (cmd); /* guaranteed by CHECK */
     error = nbd_internal_errno_of_nbd_error (error);
@@ -339,7 +356,7 @@  STATE_MACHINE {
     SET_NEXT_STATE (%.READY);
     return 0;
   case 0:
-    length = be32toh (h->sbuf.sr.structured_reply.length);
+    length = h->sbuf.sr.hdr.structured_reply.length; /* normalized in CHECK */
     offset = be64toh (h->sbuf.sr.payload.offset_data.offset);

     assert (cmd); /* guaranteed by CHECK */
@@ -377,7 +394,7 @@  STATE_MACHINE {
     SET_NEXT_STATE (%.READY);
     return 0;
   case 0:
-    length = be32toh (h->sbuf.sr.structured_reply.length);
+    length = h->sbuf.sr.hdr.structured_reply.length; /* normalized in CHECK */
     offset = be64toh (h->sbuf.sr.payload.offset_data.offset);

     assert (cmd); /* guaranteed by CHECK */
@@ -454,7 +471,7 @@  STATE_MACHINE {
     SET_NEXT_STATE (%.READY);
     return 0;
   case 0:
-    length = be32toh (h->sbuf.sr.structured_reply.length);
+    length = h->sbuf.sr.hdr.structured_reply.length; /* normalized in CHECK */

     assert (cmd); /* guaranteed by CHECK */
     assert (cmd->type == NBD_CMD_BLOCK_STATUS);
@@ -489,7 +506,7 @@  STATE_MACHINE {
     SET_NEXT_STATE (%.READY);
     return 0;
   case 0:
-    length = be32toh (h->sbuf.sr.structured_reply.length);
+    length = h->sbuf.sr.hdr.structured_reply.length; /* normalized in CHECK */

     assert (cmd); /* guaranteed by CHECK */
     assert (cmd->type == NBD_CMD_BLOCK_STATUS);
@@ -535,7 +552,7 @@  STATE_MACHINE {
  REPLY.STRUCTURED_REPLY.FINISH:
   uint16_t flags;

-  flags = be16toh (h->sbuf.sr.structured_reply.flags);
+  flags = be16toh (h->sbuf.sr.hdr.structured_reply.flags);
   if (flags & NBD_REPLY_FLAG_DONE) {
     SET_NEXT_STATE (%^FINISH_COMMAND);
   }
diff --git a/generator/states-reply.c b/generator/states-reply.c
index 9099a76a..949e982e 100644
--- a/generator/states-reply.c
+++ b/generator/states-reply.c
@@ -1,5 +1,5 @@ 
 /* nbd client library in userspace: state machine
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * Copyright (C) 2013-2021 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
@@ -68,7 +68,10 @@  STATE_MACHINE {
   assert (h->rlen == 0);

   h->rbuf = &h->sbuf;
-  h->rlen = sizeof h->sbuf.simple_reply;
+  if (h->extended_headers)
+    h->rlen = sizeof h->sbuf.simple_reply_ext;
+  else
+    h->rlen = sizeof h->sbuf.simple_reply;

   r = h->sock->ops->recv (h, h->sock, h->rbuf, h->rlen);
   if (r == -1) {
@@ -113,13 +116,27 @@  STATE_MACHINE {
   uint64_t cookie;

   magic = be32toh (h->sbuf.simple_reply.magic);
-  if (magic == NBD_SIMPLE_REPLY_MAGIC) {
+  switch (magic) {
+  case NBD_SIMPLE_REPLY_MAGIC:
+  case NBD_SIMPLE_REPLY_EXT_MAGIC:
+    if ((magic == NBD_SIMPLE_REPLY_MAGIC) == h->extended_headers)
+      goto invalid;
+    if (magic == NBD_SIMPLE_REPLY_EXT_MAGIC &&
+        (h->sbuf.simple_reply_ext.pad1 || h->sbuf.simple_reply_ext.pad2)) {
+      set_error (0, "server sent non-zero padding in simple reply header");
+      SET_NEXT_STATE (%.DEAD);
+      return 0;
+    }
     SET_NEXT_STATE (%SIMPLE_REPLY.START);
-  }
-  else if (magic == NBD_STRUCTURED_REPLY_MAGIC) {
+    break;
+  case NBD_STRUCTURED_REPLY_MAGIC:
+  case NBD_STRUCTURED_REPLY_EXT_MAGIC:
+    if ((magic == NBD_STRUCTURED_REPLY_MAGIC) == h->extended_headers)
+      goto invalid;
     SET_NEXT_STATE (%STRUCTURED_REPLY.START);
-  }
-  else {
+    break;
+  default:
+  invalid:
     SET_NEXT_STATE (%.DEAD); /* We've probably lost synchronization. */
     set_error (0, "invalid reply magic");
     return 0;