[Libguestfs] [libnbd PATCH 5/7] states: Factor out NBD_REP payload prep

Eric Blake eblake at redhat.com
Fri Jun 14 21:54:01 UTC 2019


Instead of repeating a check for valid reply headers in each sub-state
machine, let's have a common helper function do all the
work. Additionally, a central location will make it easier to
uniformly capture any NBD_REP_ERR message payloads.
---
 generator/generator                           |  8 +--
 generator/states-newstyle-opt-go.c            | 40 +++----------
 .../states-newstyle-opt-set-meta-context.c    | 39 ++-----------
 generator/states-newstyle-opt-starttls.c      | 31 ++--------
 .../states-newstyle-opt-structured-reply.c    | 28 ++-------
 generator/states-newstyle.c                   | 58 +++++++++++++++++++
 6 files changed, 86 insertions(+), 118 deletions(-)

diff --git a/generator/generator b/generator/generator
index e3dd10f..a289741 100755
--- a/generator/generator
+++ b/generator/generator
@@ -373,8 +373,8 @@ and newstyle_opt_starttls_state_machine = [

   State {
     default_state with
-    name = "SKIP_REPLY_PAYLOAD";
-    comment = "Skip newstyle NBD_OPT_STARTTLS reply payload";
+    name = "RECV_REPLY_PAYLOAD";
+    comment = "Receive any newstyle NBD_OPT_STARTTLS reply payload";
     external_events = [ NotifyRead, "" ];
   };

@@ -425,8 +425,8 @@ and newstyle_opt_structured_reply_state_machine = [

   State {
     default_state with
-    name = "SKIP_REPLY_PAYLOAD";
-    comment = "Skip newstyle NBD_OPT_STRUCTURED_REPLY reply payload";
+    name = "RECV_REPLY_PAYLOAD";
+    comment = "Receive any newstyle NBD_OPT_STRUCTURED_REPLY reply payload";
     external_events = [ NotifyRead, "" ];
   };

diff --git a/generator/states-newstyle-opt-go.c b/generator/states-newstyle-opt-go.c
index 2ee739f..3458f09 100644
--- a/generator/states-newstyle-opt-go.c
+++ b/generator/states-newstyle-opt-go.c
@@ -77,21 +77,13 @@
   return 0;

  NEWSTYLE.OPT_GO.RECV_REPLY:
-  uint32_t len;
-  const size_t maxpayload = sizeof h->sbuf.or.payload;
-
   switch (recv_into_rbuf (h)) {
   case -1: SET_NEXT_STATE (%.DEAD); return -1;
   case 0:
-    /* Read the following payload if it is short enough to fit in the
-     * static buffer.  If it's too long, skip it.
-     */
-    len = be32toh (h->sbuf.or.option_reply.replylen);
-    if (len <= maxpayload)
-      h->rbuf = &h->sbuf.or.payload;
-    else
-      h->rbuf = NULL;
-    h->rlen = len;
+    if (prepare_for_reply_payload (h, NBD_OPT_GO) == -1) {
+      SET_NEXT_STATE (%.DEAD);
+      return -1;
+    }
     SET_NEXT_STATE (%RECV_REPLY_PAYLOAD);
   }
   return 0;
@@ -104,8 +96,6 @@
   return 0;

  NEWSTYLE.OPT_GO.CHECK_REPLY:
-  uint64_t magic;
-  uint32_t option;
   uint32_t reply;
   uint32_t len;
   const size_t maxpayload = sizeof h->sbuf.or.payload;
@@ -113,34 +103,18 @@
   uint64_t exportsize;
   uint16_t eflags;

-  magic = be64toh (h->sbuf.or.option_reply.magic);
-  option = be32toh (h->sbuf.or.option_reply.option);
   reply = be32toh (h->sbuf.or.option_reply.reply);
   len = be32toh (h->sbuf.or.option_reply.replylen);
-  if (magic != NBD_REP_MAGIC || option != NBD_OPT_GO) {
-    SET_NEXT_STATE (%.DEAD);
-    set_error (0, "handshake: invalid option reply magic or option");
-    return -1;
-  }
+
   switch (reply) {
   case NBD_REP_ACK:
-    if (len != 0) {
-      SET_NEXT_STATE (%.DEAD);
-      set_error (0, "handshake: invalid option reply length");
-      return -1;
-    }
     SET_NEXT_STATE (%.READY);
     return 0;
   case NBD_REP_INFO:
-    if (len > maxpayload /* see RECV_NEWSTYLE_OPT_GO_REPLY */) {
+    if (len > maxpayload /* see RECV_NEWSTYLE_OPT_GO_REPLY */)
       debug (h, "skipping large NBD_REP_INFO");
-    }
-    else if (len < sizeof h->sbuf.or.payload.export.info) {
-      SET_NEXT_STATE (%.DEAD);
-      set_error (0, "handshake: NBD_REP_INFO reply length too small");
-      return -1;
-    }
     else {
+      assert (len >= sizeof h->sbuf.or.payload.export.info);
       info = be16toh (h->sbuf.or.payload.export.info);
       switch (info) {
       case NBD_INFO_EXPORT:
diff --git a/generator/states-newstyle-opt-set-meta-context.c b/generator/states-newstyle-opt-set-meta-context.c
index feaa295..94120e2 100644
--- a/generator/states-newstyle-opt-set-meta-context.c
+++ b/generator/states-newstyle-opt-set-meta-context.c
@@ -139,21 +139,13 @@
   return 0;

  NEWSTYLE.OPT_SET_META_CONTEXT.RECV_REPLY:
-  uint32_t len;
-  const uint32_t maxpayload = sizeof h->sbuf.or.payload.context;
-
   switch (recv_into_rbuf (h)) {
   case -1: SET_NEXT_STATE (%.DEAD); return -1;
   case 0:
-    /* Read the following payload if it is short enough to fit in the
-     * static buffer.  If it's too long, skip it.
-     */
-    len = be32toh (h->sbuf.or.option_reply.replylen);
-    if (len <= maxpayload)
-      h->rbuf = &h->sbuf.or.payload;
-    else
-      h->rbuf = NULL;
-    h->rlen = len;
+    if (prepare_for_reply_payload (h, NBD_OPT_SET_META_CONTEXT) == -1) {
+      SET_NEXT_STATE (%.DEAD);
+      return -1;
+    }
     SET_NEXT_STATE (%RECV_REPLY_PAYLOAD);
   }
   return 0;
@@ -166,43 +158,22 @@
   return 0;

  NEWSTYLE.OPT_SET_META_CONTEXT.CHECK_REPLY:
-  uint64_t magic;
-  uint32_t option;
   uint32_t reply;
   uint32_t len;
   const size_t maxpayload = sizeof h->sbuf.or.payload.context;
   struct meta_context *meta_context;

-  magic = be64toh (h->sbuf.or.option_reply.magic);
-  option = be32toh (h->sbuf.or.option_reply.option);
   reply = be32toh (h->sbuf.or.option_reply.reply);
   len = be32toh (h->sbuf.or.option_reply.replylen);
-  if (magic != NBD_REP_MAGIC || option != NBD_OPT_SET_META_CONTEXT) {
-    SET_NEXT_STATE (%.DEAD);
-    set_error (0, "handshake: invalid option reply magic or option");
-    return -1;
-  }
   switch (reply) {
   case NBD_REP_ACK:           /* End of list of replies. */
-    if (len != 0) {
-      SET_NEXT_STATE (%.DEAD);
-      set_error (0, "handshake: invalid option reply length");
-      return -1;
-    }
     SET_NEXT_STATE (%^OPT_GO.START);
     break;
   case NBD_REP_META_CONTEXT:  /* A context. */
     if (len > maxpayload)
       debug (h, "skipping too large meta context");
-    else if (len <= sizeof h->sbuf.or.payload.context.context) {
-      /* A valid reply has at least one byte in payload.context.str */
-      set_error (0, "handshake: NBD_REP_META_CONTEXT reply length too small");
-      SET_NEXT_STATE (%.DEAD);
-      return -1;
-    }
     else {
-      len = be32toh (h->sbuf.or.option_reply.replylen);
-
+      assert (len > sizeof h->sbuf.or.payload.context.context.context_id);
       meta_context = malloc (sizeof *meta_context);
       if (meta_context == NULL) {
         set_error (errno, "malloc");
diff --git a/generator/states-newstyle-opt-starttls.c b/generator/states-newstyle-opt-starttls.c
index c5eba91..e994ffd 100644
--- a/generator/states-newstyle-opt-starttls.c
+++ b/generator/states-newstyle-opt-starttls.c
@@ -45,20 +45,18 @@
   return 0;

  NEWSTYLE.OPT_STARTTLS.RECV_REPLY:
-  uint32_t len;
-
   switch (recv_into_rbuf (h)) {
   case -1: SET_NEXT_STATE (%.DEAD); return -1;
   case 0:
-    /* Discard the payload if there is one. */
-    len = be32toh (h->sbuf.or.option_reply.replylen);
-    h->rbuf = NULL;
-    h->rlen = len;
-    SET_NEXT_STATE (%SKIP_REPLY_PAYLOAD);
+    if (prepare_for_reply_payload (h, NBD_OPT_STARTTLS) == -1) {
+      SET_NEXT_STATE (%.DEAD);
+      return -1;
+    }
+    SET_NEXT_STATE (%RECV_REPLY_PAYLOAD);
   }
   return 0;

- NEWSTYLE.OPT_STARTTLS.SKIP_REPLY_PAYLOAD:
+ NEWSTYLE.OPT_STARTTLS.RECV_REPLY_PAYLOAD:
   switch (recv_into_rbuf (h)) {
   case -1: SET_NEXT_STATE (%.DEAD); return -1;
   case 0:  SET_NEXT_STATE (%CHECK_REPLY);
@@ -66,29 +64,12 @@
   return 0;

  NEWSTYLE.OPT_STARTTLS.CHECK_REPLY:
-  uint64_t magic;
-  uint32_t option;
   uint32_t reply;
-  uint32_t len;
   struct socket *new_sock;

-  magic = be64toh (h->sbuf.or.option_reply.magic);
-  option = be32toh (h->sbuf.or.option_reply.option);
   reply = be32toh (h->sbuf.or.option_reply.reply);
-  len = be32toh (h->sbuf.or.option_reply.replylen);
-  if (magic != NBD_REP_MAGIC || option != NBD_OPT_STARTTLS) {
-    SET_NEXT_STATE (%.DEAD);
-    set_error (0, "handshake: invalid option reply magic or option");
-    return -1;
-  }
   switch (reply) {
   case NBD_REP_ACK:
-    if (len != 0) {
-      SET_NEXT_STATE (%.DEAD);
-      set_error (0, "handshake: invalid option reply length");
-      return -1;
-    }
-
     new_sock = nbd_internal_crypto_create_session (h, h->sock);
     if (new_sock == NULL) {
       SET_NEXT_STATE (%.DEAD);
diff --git a/generator/states-newstyle-opt-structured-reply.c b/generator/states-newstyle-opt-structured-reply.c
index 36f9eb3..d755411 100644
--- a/generator/states-newstyle-opt-structured-reply.c
+++ b/generator/states-newstyle-opt-structured-reply.c
@@ -39,20 +39,18 @@
   return 0;

  NEWSTYLE.OPT_STRUCTURED_REPLY.RECV_REPLY:
-  uint32_t len;
-
   switch (recv_into_rbuf (h)) {
   case -1: SET_NEXT_STATE (%.DEAD); return -1;
   case 0:
-    /* Discard the payload if there is one. */
-    len = be32toh (h->sbuf.or.option_reply.replylen);
-    h->rbuf = NULL;
-    h->rlen = len;
-    SET_NEXT_STATE (%SKIP_REPLY_PAYLOAD);
+    if (prepare_for_reply_payload (h, NBD_OPT_STRUCTURED_REPLY) == -1) {
+      SET_NEXT_STATE (%.DEAD);
+      return -1;
+    }
+    SET_NEXT_STATE (%RECV_REPLY_PAYLOAD);
   }
   return 0;

- NEWSTYLE.OPT_STRUCTURED_REPLY.SKIP_REPLY_PAYLOAD:
+ NEWSTYLE.OPT_STRUCTURED_REPLY.RECV_REPLY_PAYLOAD:
   switch (recv_into_rbuf (h)) {
   case -1: SET_NEXT_STATE (%.DEAD); return -1;
   case 0:  SET_NEXT_STATE (%CHECK_REPLY);
@@ -60,25 +58,11 @@
   return 0;

  NEWSTYLE.OPT_STRUCTURED_REPLY.CHECK_REPLY:
-  uint64_t magic;
-  uint32_t option;
   uint32_t reply;

-  magic = be64toh (h->sbuf.or.option_reply.magic);
-  option = be32toh (h->sbuf.or.option_reply.option);
   reply = be32toh (h->sbuf.or.option_reply.reply);
-  if (magic != NBD_REP_MAGIC || option != NBD_OPT_STRUCTURED_REPLY) {
-    SET_NEXT_STATE (%.DEAD);
-    set_error (0, "handshake: invalid option reply magic or option");
-    return -1;
-  }
   switch (reply) {
   case NBD_REP_ACK:
-    if (h->sbuf.or.option_reply.replylen != 0) {
-      SET_NEXT_STATE (%.DEAD);
-      set_error (0, "handshake: invalid option reply length");
-      return -1;
-    }
     debug (h, "negotiated structured replies on this connection");
     h->structured_replies = true;
     break;
diff --git a/generator/states-newstyle.c b/generator/states-newstyle.c
index 6d81ab8..e86282b 100644
--- a/generator/states-newstyle.c
+++ b/generator/states-newstyle.c
@@ -16,6 +16,64 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */

+#include "internal.h"
+
+/* Common code for parsing a reply to NBD_OPT_*. */
+static int
+prepare_for_reply_payload (struct nbd_handle *h, uint32_t opt)
+{
+  const size_t maxpayload = sizeof h->sbuf.or.payload;
+  uint64_t magic;
+  uint32_t option;
+  uint32_t reply;
+  uint32_t len;
+
+  magic = be64toh (h->sbuf.or.option_reply.magic);
+  option = be32toh (h->sbuf.or.option_reply.option);
+  reply = be32toh (h->sbuf.or.option_reply.reply);
+  len = be32toh (h->sbuf.or.option_reply.replylen);
+  if (magic != NBD_REP_MAGIC || option != opt) {
+    set_error (0, "handshake: invalid option reply magic or option");
+    return -1;
+  }
+
+  /* Validate lengths that the state machine depends on. */
+  switch (reply) {
+  case NBD_REP_ACK:
+    if (len != 0) {
+      set_error (0, "handshake: invalid NBD_REP_ACK option reply length");
+      return -1;
+    }
+    break;
+  case NBD_REP_INFO:
+    /* Can't enforce an upper bound, thanks to unknown INFOs */
+    if (len < sizeof h->sbuf.or.payload.export.info) {
+      set_error (0, "handshake: NBD_REP_INFO reply length too small");
+      return -1;
+    }
+    break;
+  case NBD_REP_META_CONTEXT:
+    if (len <= sizeof h->sbuf.or.payload.context.context.context_id ||
+        len > sizeof h->sbuf.or.payload.context.context) {
+      set_error (0, "handshake: invalid NBD_REP_META_CONTEXT reply length");
+      return -1;
+    }
+    break;
+  }
+
+  /* Read the following payload if it is short enough to fit in the
+   * static buffer.  If it's too long, skip it.
+   */
+  /* XXX Move to DEAD if len > MAX_REQUEST_SIZE */
+  len = be32toh (h->sbuf.or.option_reply.replylen);
+  if (len <= maxpayload)
+    h->rbuf = &h->sbuf.or.payload;
+  else
+    h->rbuf = NULL;
+  h->rlen = len;
+  return 0;
+}
+
 /* State machine for parsing the fixed newstyle handshake. */

 /* STATE MACHINE */ {
-- 
2.20.1




More information about the Libguestfs mailing list