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

[Libguestfs] [nbdkit PATCH v2 6/7] server: Fix OPT_GO on different export than SET_META_CONTEXT



The NBD spec says that if a client requests SET_META_CONTEXT for
exportA, but later requests NBD_OPT_GO/EXPORT_NAME for exportB, then
the server should reject NBD_CMD_BLOCK_STATUS requests (that is, the
context returned for exportA need not apply to exportB).  When we
originally added base:allocation, our argument was that we always
ignore export names, so it was easier to just treat any two export
names as being the same export, so no need to reset things.  But now
that we have nbdkit_export_name(), we are better off obeying the spec.

Note that there are no known clients in the wild that can actually
perform this cross-export-name request; this was found by inspection,
but I also tested it via strategic gdb breakpoints in qemu-io.  I also
don't see the point in hacking up libnbd to become such a client.

Fixes: 4f303e44
Signed-off-by: Eric Blake <eblake redhat com>
---
 server/internal.h                    |  1 +
 server/protocol-handshake-newstyle.c | 16 +++++++++++++---
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/server/internal.h b/server/internal.h
index e6a22f1a..97e417f9 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -200,6 +200,7 @@ struct connection {
   size_t nr_handles;

   char exportname[NBD_MAX_STRING + 1];
+  uint32_t exportnamelen;
   uint32_t cflags;
   uint16_t eflags;
   bool using_tls;
diff --git a/server/protocol-handshake-newstyle.c b/server/protocol-handshake-newstyle.c
index 3b5d144e..2480d7a3 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -239,8 +239,12 @@ check_export_name (struct connection *conn, uint32_t option, char *buf,

   assert (exportnamelen < sizeof conn->exportname);
   if (save) {
+    if (exportnamelen != conn->exportnamelen ||
+        memcmp (conn->exportname, buf, exportnamelen) != 0)
+      conn->meta_context_base_allocation = false;
     memcpy (conn->exportname, buf, exportnamelen);
     conn->exportname[exportnamelen] = '\0';
+    conn->exportnamelen = exportnamelen;
   }
   debug ("newstyle negotiation: %s: client requested export '%.*s'",
          name_of_nbd_opt (option), (int) exportnamelen, buf);
@@ -451,7 +455,9 @@ negotiate_handshake_newstyle_options (struct connection *conn)
         }

         /* As with NBD_OPT_EXPORT_NAME we print the export name and
-         * save it in the connection.
+         * save it in the connection.  If an earlier
+         * NBD_OPT_SET_META_CONTEXT used an export name, it must match
+         * or else we drop the support for that context.
          */
         if (check_export_name (conn, option, &data[4], exportnamelen,
                                optlen - 6, true) == -1)
@@ -574,12 +580,16 @@ negotiate_handshake_newstyle_options (struct connection *conn)
           continue;
         }

-        /* Discard the export name, after validating it. */
+        /* Remember the export name: the NBD spec says that if the client
+         * later uses NBD_OPT_GO on a different export, then the context
+         * returned here is not usable.
+         */
         memcpy (&exportnamelen, &data[0], 4);
         exportnamelen = be32toh (exportnamelen);
         what = "validating export name";
         if (check_export_name (conn, option, &data[4], exportnamelen,
-                               optlen - 8, false) == -1)
+                               optlen - 8,
+                               option == NBD_OPT_SET_META_CONTEXT) == -1)
           goto opt_meta_invalid_option_len;
         opt_index = 4 + exportnamelen;

-- 
2.21.0


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