thr3ads.net - Libguestfs - [Libguestfs] [nbdkit PATCH v2 7/7] server: Better newstyle .open failure handling [Sep 2019]

If this information is useful, please help other people find it:
Share via:

Eric Blake

2019-Sep-28 04:48 UTC

[Libguestfs] [nbdkit PATCH v2 0/7] Spec compliance patches

Since the v1 series (0/4, at [1]), I've applied patches 1 and 2,
rewritten patch 3 [Forbid NUL in export and context names] into patch
4 here, patch 4 there turned into patch 6 here, and everything else
here is new.

[1]https://www.redhat.com/archives/libguestfs/2019-September/msg00180.html

I don't know if there is a handy reusable function for checking
whether a string contains valid UTF-8 while still complying with our
licensing, so for now I don't bother to check that.  But fixing this
meant I also proposed a qemu 4.2 patch on export name length:
https://lists.gnu.org/archive/html/qemu-devel/2019-09/msg06925.html so
part of the test I added only really covers nbdkit when using qemu
4.1, because otherwise qemu fails early without involving nbdkit.

Eric Blake (7):
  server: Reject -e with too long of a name
  server: Expose -e in $exportname during --run
  server: Switch to fixed-length conn->exportname
  server: Improve check of string validity
  server: Allow longer NBD_OPT
  server: Fix OPT_GO on different export than SET_META_CONTEXT
  server: Better newstyle .open failure handling

 docs/nbdkit-captive.pod              |  12 ++-
 server/internal.h                    |   4 +-
 server/Makefile.am                   |   1 +
 server/captive.c                     |  22 ++++-
 server/connections.c                 |   7 --
 server/main.c                        |   6 ++
 server/protocol-handshake-newstyle.c | 117 ++++++++++++++++++++-------
 server/protocol-handshake-oldstyle.c |   3 +
 tests/Makefile.am                    |   2 +
 tests/test-long-name.sh              | 101 +++++++++++++++++++++++
 10 files changed, 231 insertions(+), 44 deletions(-)
 create mode 100755 tests/test-long-name.sh

-- 
2.21.0

Eric Blake

2019-Sep-28 04:48 UTC

head link

[Libguestfs] [nbdkit PATCH v2 1/7] server: Reject -e with too long of a name

The NBD protocol requires strings to be capped at 4k; we violated that
if the client requests NBD_OPT_LIST but the command line provided too
long of a string.

The protocol also requires that strings be valid UTF-8, but for now,
we are accepting any byte sequence.

However, we still need another patch before the test is fully
complete: qemu-nbd --list wants to send a valid NBD_OPT_INFO with
length longer than we currently permit.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 server/main.c           |  6 ++++
 tests/Makefile.am       |  2 ++
 tests/test-long-name.sh | 65 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 73 insertions(+)
 create mode 100755 tests/test-long-name.sh

diff --git a/server/main.c b/server/main.c
index 975716dc..56231497 100644
--- a/server/main.c
+++ b/server/main.c
@@ -51,6 +51,7 @@
 #include <dlfcn.h>

 #include "internal.h"
+#include "nbd-protocol.h"
 #include "options.h"
 #include "exit-with-parent.h"

@@ -330,6 +331,11 @@ main (int argc, char *argv[])

     case 'e':
       exportname = optarg;
+      if (strnlen (exportname, NBD_MAX_STRING + 1) > NBD_MAX_STRING) {
+        nbdkit_error ("export name too long");
+        exit (EXIT_FAILURE);
+      }
+      /* TODO: Check that name is valid UTF-8? */
       newstyle = true;
       break;

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7d254be9..046e219a 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -79,6 +79,7 @@ EXTRA_DIST = \
 	test-linuxdisk.sh \
 	test-linuxdisk-copy-out.sh \
 	test-log.sh \
+	test-long-name.sh \
 	test.lua \
 	test-memory-largest.sh \
 	test-memory-largest-for-qemu.sh \
@@ -198,6 +199,7 @@ TESTS += \
 	test-socket-activation \
 	test-foreground.sh \
 	test-debug-flags.sh \
+	test-long-name.sh \
 	$(NULL)

 check_PROGRAMS += \
diff --git a/tests/test-long-name.sh b/tests/test-long-name.sh
new file mode 100755
index 00000000..214a5e7a
--- /dev/null
+++ b/tests/test-long-name.sh
@@ -0,0 +1,65 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright (C) 2019 Red Hat Inc.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+source ./functions.sh
+set -e
+set -x
+
+fail=0
+
+# Test handling of NBD maximum string length of 4k.
+
+requires qemu-nbd --version
+
+name16=1234567812345678
+name64=$name16$name16$name16$name16
+name256=$name64$name64$name64$name64
+name1k=$name256$name256$name256$name256
+name4k=$name1k$name1k$name1k$name1k
+almost4k=${name4k%8$name16}
+
+# Test behavior of -e: accept 4k max, reject longer
+nbdkit -U - -e $name4k null --run true || fail=1
+nbdkit -U - -e a$name4k null --run true && fail=1
+
+# The rest of this test uses the ‘qemu-nbd --list’ option added in qemu 4.0.
+if ! qemu-nbd --help | grep -sq -- --list; then
+    echo "$0: skipping because qemu-nbd does not support the --list
option"
+    exit 77
+fi
+
+# Test response to NBD_OPT_LIST
+nbdkit -U - -e $almost4k null --run 'qemu-nbd --list -k $unixsocket' ||
fail=1
+# FIXME: Right now, we can't accept full 4k length - this should succeed
+nbdkit -U - -e $name4k null --run 'qemu-nbd --list -k $unixsocket'
&& fail=1
+
+exit $fail
-- 
2.21.0

Eric Blake

2019-Sep-28 04:48 UTC

head link

[Libguestfs] [nbdkit PATCH v2 2/7] server: Expose -e in $exportname during --run

Now that the export name can affect contents, we should let --run
favor connecting to the preferred export name passed to nbdkit via the
-e command line option.  In addition to exposing $exportname, update
the recently-added $uri to include the exportname, but leave the older
$nbd alone.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 docs/nbdkit-captive.pod | 12 ++++++++++--
 server/captive.c        | 22 ++++++++++++++++++----
 tests/test-long-name.sh | 20 ++++++++++++++++++++
 3 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/docs/nbdkit-captive.pod b/docs/nbdkit-captive.pod
index 0adf0a0b..9c7044f1 100644
--- a/docs/nbdkit-captive.pod
+++ b/docs/nbdkit-captive.pod
@@ -45,12 +45,14 @@ The following shell variables are available in the
I<--run> argument:
 =item C<$uri>

 A URI that refers to the nbdkit port or socket in the preferred form
-documented by the NBD project.
+documented by the NBD project.  If nbdkit was started with the B<-e>
+option to set the preferred export name, this is included in the URI.

 =item C<$nbd>

 An older URL that refers to the nbdkit port or socket in a manner
-specific to certain tools.
+specific to certain tools.  This form does not include an export name,
+even if B<-e> was used.

 Note there is some magic here, since qemu and guestfish URLs have a
 different format, so nbdkit tries to guess which you are running.  If
@@ -64,6 +66,12 @@ If E<ne> "", the port number that nbdkit is
listening on.

 If E<ne> "", the Unix domain socket that nbdkit is listening
on.

+=item C<$exportname>
+
+The default export name (which may be "") that nbdkit will advertise
+in response to NBD_OPT_LIST.  This comes from the B<-e>
+(B<--exportname>) command line option.
+
 =back

 I<--run> implies I<--foreground>.  It is not possible, and probably
diff --git a/server/captive.c b/server/captive.c
index 90e42050..c4cec238 100644
--- a/server/captive.c
+++ b/server/captive.c
@@ -71,12 +71,26 @@ run_command (void)
   if (port) {
     fprintf (fp, "nbd://localhost:");
     shell_quote (port, fp);
+    if (exportname) {
+      putc ('/', fp);
+      uri_quote (exportname, fp);
+    }
   }
   else if (unixsocket) {
-    fprintf (fp, "nbd+unix://\\?socket=");
+    fprintf (fp, "nbd+unix://");
+    if (exportname) {
+      putc ('/', fp);
+      uri_quote (exportname, fp);
+    }
+    fprintf (fp, "\\?socket=");
     uri_quote (unixsocket, fp);
   }
-  fprintf (fp, "\n");
+  putc ('\n', fp);
+
+  /* Expose $exportname. */
+  fprintf (fp, "exportname=");
+  shell_quote (exportname, fp);
+  putc ('\n', fp);

   /* Construct older $nbd "URL".  Unfortunately guestfish and qemu
take
    * different syntax, so try to guess which one we need.
@@ -106,13 +120,13 @@ run_command (void)
     else
       abort ();
   }
-  fprintf (fp, "\n");
+  putc ('\n', fp);

   /* Construct $port and $unixsocket. */
   fprintf (fp, "port=");
   if (port)
     shell_quote (port, fp);
-  fprintf (fp, "\n");
+  putc ('\n', fp);
   fprintf (fp, "unixsocket=");
   if (unixsocket)
     shell_quote (unixsocket, fp);
diff --git a/tests/test-long-name.sh b/tests/test-long-name.sh
index 214a5e7a..7b0b43ad 100755
--- a/tests/test-long-name.sh
+++ b/tests/test-long-name.sh
@@ -51,6 +51,26 @@ almost4k=${name4k%8$name16}
 nbdkit -U - -e $name4k null --run true || fail=1
 nbdkit -U - -e a$name4k null --run true && fail=1

+# Test that $exportname and $uri reflect the name
+out=$(nbdkit -U - -e $name4k null --run 'echo $exportname')
+if test "$name4k" != "$out"; then
+    echo "$0: \$exportname contains wrong contents" >&2
+    fail=1
+fi
+out=$(nbdkit -U - -e $name4k null --run 'echo $uri')
+case $out in
+    nbd+unix:///$name4k\?socket=*) ;;
+    *) echo "$0: \$uri contains wrong contents" >&2
+       fail=1 ;;
+esac
+pick_unused_port
+out=$(nbdkit -i localhost -p $port -e $name4k null --run 'echo $uri')
+case $out in
+    nbd://localhost:$port/$name4k) ;;
+    *) echo "$0: \$uri contains wrong contents" >&2
+       fail=1 ;;
+esac
+
 # The rest of this test uses the ‘qemu-nbd --list’ option added in qemu 4.0.
 if ! qemu-nbd --help | grep -sq -- --list; then
     echo "$0: skipping because qemu-nbd does not support the --list
option"
-- 
2.21.0

Eric Blake

2019-Sep-28 04:48 UTC

head link

[Libguestfs] [nbdkit PATCH v2 3/7] server: Switch to fixed-length conn->exportname

Since each connection already has a malloc()d struct conn, and the NBD
protocol places a fixed length on the maximum string at 4k, it's not
that much harder to just directly set aside 4k in the connection
struct instead of allocating a name on the fly.  In the common case of
short names, it does mean that a connection now occupies slightly more
unused heap space, but these days, 4k overhead is probably in the
noise.  Doing this makes it easier for an upcoming patch to refactor
name validation without having to worry about an allocation failure,
which in turn makes it easier to keep the connection alive even after
failing NBD_OPT_GO (where an allocation failure would have to be
handled differently than the diagnosis of an invalid client request).

This also points out a spot where we need to enlarge
MAX_OPTION_LENGTH, but that will be a separate patch.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 server/internal.h                    |  3 ++-
 server/Makefile.am                   |  1 +
 server/connections.c                 |  7 -------
 server/protocol-handshake-newstyle.c | 17 +++++------------
 4 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/server/internal.h b/server/internal.h
index fbabce69..e6a22f1a 100644
--- a/server/internal.h
+++ b/server/internal.h
@@ -43,6 +43,7 @@
 #include "nbdkit-plugin.h"
 #include "nbdkit-filter.h"
 #include "cleanup.h"
+#include "nbd-protocol.h"

 #ifdef __APPLE__
 #define UNIX_PATH_MAX 104
@@ -198,7 +199,7 @@ struct connection {
   struct b_conn_handle *handles;
   size_t nr_handles;

-  char *exportname;
+  char exportname[NBD_MAX_STRING + 1];
   uint32_t cflags;
   uint16_t eflags;
   bool using_tls;
diff --git a/server/Makefile.am b/server/Makefile.am
index a0417639..c846fc7a 100644
--- a/server/Makefile.am
+++ b/server/Makefile.am
@@ -134,6 +134,7 @@ test_public_SOURCES = \
 test_public_CPPFLAGS = \
 	-I$(top_srcdir)/include \
 	-I$(top_srcdir)/common/include \
+	-I$(top_srcdir)/common/protocol \
 	-I$(top_srcdir)/common/utils \
 	$(NULL)
 test_public_CFLAGS = $(WARNINGS_CFLAGS) $(VALGRIND_CFLAGS)
diff --git a/server/connections.c b/server/connections.c
index eeda85d5..27cf202b 100644
--- a/server/connections.c
+++ b/server/connections.c
@@ -258,11 +258,6 @@ new_connection (int sockin, int sockout, int nworkers)

   conn->status_pipe[0] = conn->status_pipe[1] = -1;

-  conn->exportname = strdup ("");
-  if (conn->exportname == NULL) {
-    perror ("strdup");
-    goto error;
-  }
   conn->handles = calloc (backend->i + 1, sizeof *conn->handles);
   if (conn->handles == NULL) {
     perror ("malloc");
@@ -335,7 +330,6 @@ new_connection (int sockin, int sockout, int nworkers)
     close (conn->status_pipe[0]);
   if (conn->status_pipe[1] >= 0)
     close (conn->status_pipe[1]);
-  free (conn->exportname);
   free (conn->handles);
   free (conn);
   return NULL;
@@ -383,7 +377,6 @@ free_connection (struct connection *conn)
   pthread_mutex_destroy (&conn->status_lock);

   free (conn->handles);
-  free (conn->exportname);
   free (conn);
 }

diff --git a/server/protocol-handshake-newstyle.c
b/server/protocol-handshake-newstyle.c
index d0fb4dd7..0cbcc3d5 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -277,12 +277,7 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
       data[optlen] = '\0';
       debug ("newstyle negotiation: %s: client requested export
'%s'",
              name_of_nbd_opt (option), data);
-      free (conn->exportname);
-      conn->exportname = malloc (optlen+1);
-      if (conn->exportname == NULL) {
-        nbdkit_error ("malloc: %m");
-        return -1;
-      }
+      assert (optlen < sizeof conn->exportname);
       strcpy (conn->exportname, data);

       /* We have to finish the handshake by sending handshake_finish. */
@@ -420,12 +415,10 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
         /* As with NBD_OPT_EXPORT_NAME we print the export name and
          * save it in the connection.
          */
-        free (conn->exportname);
-        conn->exportname = malloc (exportnamelen+1);
-        if (conn->exportname == NULL) {
-          nbdkit_error ("malloc: %m");
-          return -1;
-        }
+        /* FIXME: Our current MAX_OPTION_LENGTH prevents us from receiving
+         * an export name at the full NBD_MAX_STRING length.
+         */
+        assert (exportnamelen < sizeof conn->exportname);
         memcpy (conn->exportname, &data[4], exportnamelen);
         conn->exportname[exportnamelen] = '\0';
         debug ("newstyle negotiation: %s: client requested export
'%s'",
-- 
2.21.0

Eric Blake

2019-Sep-28 04:48 UTC

head link

[Libguestfs] [nbdkit PATCH v2 4/7] server: Improve check of string validity

Factor out a common function for checking whether a client export name
is valid, checking for length and no embedded NULs (in the future, we
may also add a check for well-formed UTF-8).  Checking for embedded
NUL is important now that we allow plugins to alter content based on
the client's request: if the client requested 'a\0b' (against the
NBD
protocol), they would be surprised if the reflection plugin only gave
back 'a'.  Rejecting bad strings is easier than altering our API to
pass through lengths to preserve embedded NUL.

Use the new functions to simplify existing code in NBD_OPT_EXPORT_NAME
and NBD_OPT_GO, but also to validate the export name handed to
NBD_OPT_SET_META_CONTEXT.

No known existing client makes it easy to send 'a\0b' as an export or
meta context name against the protocol, but well-place breakpoints in
gdb on a client process allowed me to test this manually.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 server/protocol-handshake-newstyle.c | 62 ++++++++++++++++++++++------
 tests/test-long-name.sh              | 18 ++++++++
 2 files changed, 67 insertions(+), 13 deletions(-)

diff --git a/server/protocol-handshake-newstyle.c
b/server/protocol-handshake-newstyle.c
index 0cbcc3d5..34958360 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -210,6 +210,43 @@ finish_newstyle_options (struct connection *conn, uint64_t
*exportsize)
   return 0;
 }

+static int
+check_string (uint32_t option, char *buf, uint32_t len, uint32_t maxlen,
+              const char *name)
+{
+  if (len > NBD_MAX_STRING || len > maxlen) {
+    nbdkit_error ("%s: %s too long", name_of_nbd_opt (option), name);
+    return -1;
+  }
+  if (strnlen (buf, len) != len) {
+    nbdkit_error ("%s: %s may not include NUL bytes",
+                  name_of_nbd_opt (option), name);
+    return -1;
+  }
+  /* TODO: Check for valid UTF-8? */
+  return 0;
+}
+
+/* Sub-function of negotiate_handshake_newstyle_options, to grab and
+ * validate an export name.
+ */
+static int
+check_export_name (struct connection *conn, uint32_t option, char *buf,
+                   uint32_t exportnamelen, uint32_t maxlen, bool save)
+{
+  if (check_string (option, buf, exportnamelen, maxlen, "export
name") == -1)
+    return -1;
+
+  assert (exportnamelen < sizeof conn->exportname);
+  if (save) {
+    memcpy (conn->exportname, buf, exportnamelen);
+    conn->exportname[exportnamelen] = '\0';
+  }
+  debug ("newstyle negotiation: %s: client requested export
'%.*s'",
+         name_of_nbd_opt (option), (int) exportnamelen, buf);
+  return 0;
+}
+
 static int
 negotiate_handshake_newstyle_options (struct connection *conn)
 {
@@ -273,12 +310,8 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
       if (conn_recv_full (conn, data, optlen,
                           "read: %s: %m", name_of_nbd_opt (option))
== -1)
         return -1;
-      /* Print the export name and save it in the connection. */
-      data[optlen] = '\0';
-      debug ("newstyle negotiation: %s: client requested export
'%s'",
-             name_of_nbd_opt (option), data);
-      assert (optlen < sizeof conn->exportname);
-      strcpy (conn->exportname, data);
+      if (check_export_name (conn, option, data, optlen, optlen, true) == -1)
+        return -1;

       /* We have to finish the handshake by sending handshake_finish. */
       if (finish_newstyle_options (conn, &exportsize) == -1)
@@ -418,11 +451,9 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
         /* FIXME: Our current MAX_OPTION_LENGTH prevents us from receiving
          * an export name at the full NBD_MAX_STRING length.
          */
-        assert (exportnamelen < sizeof conn->exportname);
-        memcpy (conn->exportname, &data[4], exportnamelen);
-        conn->exportname[exportnamelen] = '\0';
-        debug ("newstyle negotiation: %s: client requested export
'%s'",
-               optname, conn->exportname);
+        if (check_export_name (conn, option, &data[4], exportnamelen,
+                               optlen - 6, true) == -1)
+          return -1;

         /* The spec is confusing, but it is required that we send back
          * NBD_INFO_EXPORT, even if the client did not request it!
@@ -541,9 +572,13 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
           continue;
         }

-        /* Discard the export name. */
+        /* Discard the export name, after validating it. */
         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)
+          goto opt_meta_invalid_option_len;
         opt_index = 4 + exportnamelen;

         /* Read the number of queries. */
@@ -583,7 +618,8 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
             querylen = be32toh (querylen);
             opt_index += 4;
             what = "reading query string";
-            if (opt_index + querylen > optlen)
+            if (check_string (option, &data[opt_index], querylen,
+                              optlen - opt_index, "meta context
query") == -1)
               goto opt_meta_invalid_option_len;

             debug ("newstyle negotiation: %s: %s %.*s",
diff --git a/tests/test-long-name.sh b/tests/test-long-name.sh
index 7b0b43ad..86aefbaf 100755
--- a/tests/test-long-name.sh
+++ b/tests/test-long-name.sh
@@ -38,6 +38,7 @@ fail=0

 # Test handling of NBD maximum string length of 4k.

+requires qemu-io --version
 requires qemu-nbd --version

 name16=1234567812345678
@@ -71,6 +72,23 @@ case $out in
        fail=1 ;;
 esac

+# Use largest possible export name, then oversize, with NBD_OPT_EXPORT_NAME.
+nbdkit -U - --mask-handshake=0 null --run 'qemu-io -r -f raw -c quit \
+  nbd+unix:///'$name4k'\?socket=$unixsocket' || fail=1
+# qemu 4.1 did not length check, letting it send an invalid NBD client
+# request which nbdkit must filter out. Later qemu might refuse to
+# send the request (like libnbd does), at which point this is no longer
+# testing nbdkit proper, so we may remove it later:
+nbdkit -U - --mask-handshake=0 null --run 'qemu-io -r -f raw -c quit \
+  nbd+unix:///'a$name4k'\?socket=$unixsocket' && fail=1
+
+# Repeat with NBD_OPT_GO.
+nbdkit -U - null --run 'qemu-io -r -f raw -c quit \
+  nbd+unix:///'$name1k$name1k'\?socket=$unixsocket' || fail=1
+# FIXME: Right now, we can't accept full 4k length - this should succeed
+nbdkit -U - null --run 'qemu-io -r -f raw -c quit \
+  nbd+unix:///'$almost4k'\?socket=$unixsocket' && fail=1
+
 # The rest of this test uses the ‘qemu-nbd --list’ option added in qemu 4.0.
 if ! qemu-nbd --help | grep -sq -- --list; then
     echo "$0: skipping because qemu-nbd does not support the --list
option"
-- 
2.21.0

Eric Blake

2019-Sep-28 04:48 UTC

head link

[Libguestfs] [nbdkit PATCH v2 5/7] server: Allow longer NBD_OPT

Fixes the fact that clients could not request the maximum string
length except with NBD_OPT_EXPORT_LEN.  Updates the testsuite to
match.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 server/protocol-handshake-newstyle.c | 12 +++++++-----
 tests/test-long-name.sh              | 10 ++++------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/server/protocol-handshake-newstyle.c
b/server/protocol-handshake-newstyle.c
index 34958360..3b5d144e 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -48,7 +48,7 @@
 #define MAX_NR_OPTIONS 32

 /* Maximum length of any option data (bytes). */
-#define MAX_OPTION_LENGTH 4096
+#define MAX_OPTION_LENGTH (NBD_MAX_STRING * 4)

 /* Receive newstyle options. */
 static int
@@ -255,7 +255,7 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
   uint64_t version;
   uint32_t option;
   uint32_t optlen;
-  char data[MAX_OPTION_LENGTH+1];
+  CLEANUP_FREE char *data = NULL;
   struct nbd_export_name_option_reply handshake_finish;
   const char *optname;
   uint64_t exportsize;
@@ -281,6 +281,11 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
       nbdkit_error ("client option data too long (%" PRIu32
")", optlen);
       return -1;
     }
+    data = malloc (optlen + 1); /* Allowing a trailing NUL helps some uses */
+    if (data == NULL) {
+      nbdkit_error ("malloc: %m");
+      return -1;
+    }

     option = be32toh (new_option.option);
     optname = name_of_nbd_opt (option);
@@ -448,9 +453,6 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
         /* As with NBD_OPT_EXPORT_NAME we print the export name and
          * save it in the connection.
          */
-        /* FIXME: Our current MAX_OPTION_LENGTH prevents us from receiving
-         * an export name at the full NBD_MAX_STRING length.
-         */
         if (check_export_name (conn, option, &data[4], exportnamelen,
                                optlen - 6, true) == -1)
           return -1;
diff --git a/tests/test-long-name.sh b/tests/test-long-name.sh
index 86aefbaf..f9ebad6e 100755
--- a/tests/test-long-name.sh
+++ b/tests/test-long-name.sh
@@ -84,10 +84,10 @@ nbdkit -U - --mask-handshake=0 null --run 'qemu-io -r -f
raw -c quit \

 # Repeat with NBD_OPT_GO.
 nbdkit -U - null --run 'qemu-io -r -f raw -c quit \
-  nbd+unix:///'$name1k$name1k'\?socket=$unixsocket' || fail=1
-# FIXME: Right now, we can't accept full 4k length - this should succeed
+  nbd+unix:///'$name4k'\?socket=$unixsocket' || fail=1
+# See above comment about whether this is testing nbdkit or qemu:
 nbdkit -U - null --run 'qemu-io -r -f raw -c quit \
-  nbd+unix:///'$almost4k'\?socket=$unixsocket' && fail=1
+  nbd+unix:///'a$name4k'\?socket=$unixsocket' && fail=1

 # The rest of this test uses the ‘qemu-nbd --list’ option added in qemu 4.0.
 if ! qemu-nbd --help | grep -sq -- --list; then
@@ -96,8 +96,6 @@ if ! qemu-nbd --help | grep -sq -- --list; then
 fi

 # Test response to NBD_OPT_LIST
-nbdkit -U - -e $almost4k null --run 'qemu-nbd --list -k $unixsocket' ||
fail=1
-# FIXME: Right now, we can't accept full 4k length - this should succeed
-nbdkit -U - -e $name4k null --run 'qemu-nbd --list -k $unixsocket'
&& fail=1
+nbdkit -U - -e $name4k null --run 'qemu-nbd --list -k $unixsocket' ||
fail=1

 exit $fail
-- 
2.21.0

Eric Blake

2019-Sep-28 04:48 UTC

head link

[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

Eric Blake

2019-Sep-28 04:48 UTC

head link

[Libguestfs] [nbdkit PATCH v2 7/7] server: Better newstyle .open failure handling

If a plugin's .open or .get_size or .can_write fails, right now that
is fatal to the connection.  When nbdkit was first implemented, this
made sense (there was no way to report errors to oldstyle or
NBD_OPT_EXPORT_NAME).  But now that newstyle is around, it's rather
abrupt to hang up on the client, and better is to return an error to
NBD_OPT_GO, and let the client choose what to do (most clients will
probably hang up, whether gracefully with NBD_OPT_ABORT or abruptly,
rather than try other NBD_OPT_*, but _we_ shouldn't be the ones
forcing their hand).

For an example of what this improves, if you run:

$ nbdkit -fv sh - <<\EOF
case $1 in get_size) echo oops >&2; exit 1;; *) exit 2;; esac
EOF

Pre-patch, qemu complains about the abrupt server death:
$ qemu-nbd --list
qemu-nbd: Failed to read option reply: Unexpected end-of-file before all bytes
were read

Post-patch, qemu gets the desired error message, and exits gracefully:
$ qemu-nbd --list
qemu-nbd: Requested export not available

Note that this does not fix the pre-existing problem that we can end
up calling .finalize even when .prepare was skipped or failed (that
latent problem was first exposed for the rare client that calls
NBD_OPT_INFO before NBD_OPT_GO, see commit a6b88b19); a later patch
will have to add better bookkeeping for that, and for better handling
of reopen requests from the retry filter.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 server/protocol-handshake-newstyle.c | 32 +++++++++++++++++++++-------
 server/protocol-handshake-oldstyle.c |  3 +++
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/server/protocol-handshake-newstyle.c
b/server/protocol-handshake-newstyle.c
index 2480d7a3..878fe53f 100644
--- a/server/protocol-handshake-newstyle.c
+++ b/server/protocol-handshake-newstyle.c
@@ -198,7 +198,7 @@ conn_recv_full (struct connection *conn, void *buf, size_t
len,

 /* Sub-function of negotiate_handshake_newstyle_options below.  It
  * must be called on all non-error paths out of the options for-loop
- * in that function.
+ * in that function, and must not cause any wire traffic.
  */
 static int
 finish_newstyle_options (struct connection *conn, uint64_t *exportsize)
@@ -322,7 +322,9 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
       if (check_export_name (conn, option, data, optlen, optlen, true) == -1)
         return -1;

-      /* We have to finish the handshake by sending handshake_finish. */
+      /* We have to finish the handshake by sending handshake_finish.
+       * On failure, we have to disconnect.
+       */
       if (finish_newstyle_options (conn, &exportsize) == -1)
         return -1;

@@ -460,16 +462,30 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
          * or else we drop the support for that context.
          */
         if (check_export_name (conn, option, &data[4], exportnamelen,
-                               optlen - 6, true) == -1)
-          return -1;
+                               optlen - 6, true) == -1) {
+          if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID)
+              == -1)
+            return -1;
+          continue;
+        }

         /* The spec is confusing, but it is required that we send back
          * NBD_INFO_EXPORT, even if the client did not request it!
          * qemu client in particular does not request this, but will
-         * fail if we don't send it.
+         * fail if we don't send it.  Note that if .open fails, but we
+         * succeed at .close, then we merely return an error to the
+         * client and let them try another NBD_OPT, rather than
+         * disconnecting.
          */
-        if (finish_newstyle_options (conn, &exportsize) == -1)
-          return -1;
+        if (finish_newstyle_options (conn, &exportsize) == -1) {
+          if (backend->finalize (backend, conn) == -1)
+            return -1;
+          backend_close (backend, conn);
+          if (send_newstyle_option_reply (conn, option,
+                                          NBD_REP_ERR_UNKNOWN) == -1)
+            return -1;
+          continue;
+        }

         if (send_newstyle_option_reply_info_export (conn, option,
                                                     NBD_REP_INFO,
@@ -497,7 +513,7 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
       }

       /* Unlike NBD_OPT_EXPORT_NAME, NBD_OPT_GO sends back an ACK
-       * or ERROR packet.
+       * or ERROR packet.  If this was NBD_OPT_LIST, call .close.
        */
       if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1)
         return -1;
diff --git a/server/protocol-handshake-oldstyle.c
b/server/protocol-handshake-oldstyle.c
index 4efc668b..45a1a486 100644
--- a/server/protocol-handshake-oldstyle.c
+++ b/server/protocol-handshake-oldstyle.c
@@ -52,6 +52,9 @@ protocol_handshake_oldstyle (struct connection *conn)

   assert (tls != 2); /* Already filtered in main */

+  /* With oldstyle, our only option if .open or friends fail is to
+   * disconnect, as we cannot report the problem to the client.
+   */
   if (protocol_common_open (conn, &exportsize, &eflags) == -1)
     return -1;

-- 
2.21.0

Richard W.M. Jones

2019-Sep-28 20:07 UTC

head link

Re: [Libguestfs] [nbdkit PATCH v2 5/7] server: Allow longer NBD_OPT

On Fri, Sep 27, 2019 at 11:48:47PM -0500, Eric Blake
wrote:> Fixes the fact that clients could not request the maximum string
> length except with NBD_OPT_EXPORT_LEN.  Updates the testsuite to
> match.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  server/protocol-handshake-newstyle.c | 12 +++++++-----
>  tests/test-long-name.sh              | 10 ++++------
>  2 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/server/protocol-handshake-newstyle.c
b/server/protocol-handshake-newstyle.c
> index 34958360..3b5d144e 100644
> --- a/server/protocol-handshake-newstyle.c
> +++ b/server/protocol-handshake-newstyle.c
> @@ -48,7 +48,7 @@
>  #define MAX_NR_OPTIONS 32
> 
>  /* Maximum length of any option data (bytes). */
> -#define MAX_OPTION_LENGTH 4096
> +#define MAX_OPTION_LENGTH (NBD_MAX_STRING * 4)
I may have missed it - why was * 4 chosen?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

Richard W.M. Jones

2019-Sep-28 20:08 UTC

head link

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

I'm good up to patch 6.  I had a small comment about patch 5, but

ACK

I'll look at patch 7 later.

Thanks,
Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

Richard W.M. Jones

2019-Sep-28 20:28 UTC

head link

Re: [Libguestfs] [nbdkit PATCH v2 7/7] server: Better newstyle .open failure handling

On Fri, Sep 27, 2019 at 11:48:49PM -0500, Eric Blake
wrote:> If a plugin's .open or .get_size or .can_write fails, right now that
> is fatal to the connection.  When nbdkit was first implemented, this
> made sense (there was no way to report errors to oldstyle or
> NBD_OPT_EXPORT_NAME).  But now that newstyle is around, it's rather
> abrupt to hang up on the client, and better is to return an error to
> NBD_OPT_GO, and let the client choose what to do (most clients will
> probably hang up, whether gracefully with NBD_OPT_ABORT or abruptly,
> rather than try other NBD_OPT_*, but _we_ shouldn't be the ones
> forcing their hand).
> 
> For an example of what this improves, if you run:
> 
> $ nbdkit -fv sh - <<\EOF
> case $1 in get_size) echo oops >&2; exit 1;; *) exit 2;; esac
> EOF
> 
> Pre-patch, qemu complains about the abrupt server death:
> $ qemu-nbd --list
> qemu-nbd: Failed to read option reply: Unexpected end-of-file before all
bytes were read
> 
> Post-patch, qemu gets the desired error message, and exits gracefully:
> $ qemu-nbd --list
> qemu-nbd: Requested export not available
> 
> Note that this does not fix the pre-existing problem that we can end
> up calling .finalize even when .prepare was skipped or failed (that
> latent problem was first exposed for the rare client that calls
> NBD_OPT_INFO before NBD_OPT_GO, see commit a6b88b19); a later patch
> will have to add better bookkeeping for that, and for better handling
> of reopen requests from the retry filter.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  server/protocol-handshake-newstyle.c | 32 +++++++++++++++++++++-------
>  server/protocol-handshake-oldstyle.c |  3 +++
>  2 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/server/protocol-handshake-newstyle.c
b/server/protocol-handshake-newstyle.c
> index 2480d7a3..878fe53f 100644
> --- a/server/protocol-handshake-newstyle.c
> +++ b/server/protocol-handshake-newstyle.c
> @@ -198,7 +198,7 @@ conn_recv_full (struct connection *conn, void *buf,
size_t len,
> 
>  /* Sub-function of negotiate_handshake_newstyle_options below.  It
>   * must be called on all non-error paths out of the options for-loop
> - * in that function.
> + * in that function, and must not cause any wire traffic.
>   */
>  static int
>  finish_newstyle_options (struct connection *conn, uint64_t *exportsize)
> @@ -322,7 +322,9 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
>        if (check_export_name (conn, option, data, optlen, optlen, true) ==
-1)
>          return -1;
> 
> -      /* We have to finish the handshake by sending handshake_finish. */
> +      /* We have to finish the handshake by sending handshake_finish.
> +       * On failure, we have to disconnect.
> +       */
>        if (finish_newstyle_options (conn, &exportsize) == -1)
>          return -1;
> 
> @@ -460,16 +462,30 @@ negotiate_handshake_newstyle_options (struct
connection *conn)
>           * or else we drop the support for that context.
>           */
>          if (check_export_name (conn, option, &data[4], exportnamelen,
> -                               optlen - 6, true) == -1)
> -          return -1;
> +                               optlen - 6, true) == -1) {
> +          if (send_newstyle_option_reply (conn, option,
NBD_REP_ERR_INVALID)
> +              == -1)
> +            return -1;
> +          continue;
> +        }
> 
>          /* The spec is confusing, but it is required that we send back
>           * NBD_INFO_EXPORT, even if the client did not request it!
>           * qemu client in particular does not request this, but will
> -         * fail if we don't send it.
> +         * fail if we don't send it.  Note that if .open fails, but we
> +         * succeed at .close, then we merely return an error to the
> +         * client and let them try another NBD_OPT, rather than
> +         * disconnecting.
>           */
> -        if (finish_newstyle_options (conn, &exportsize) == -1)
> -          return -1;
> +        if (finish_newstyle_options (conn, &exportsize) == -1) {
> +          if (backend->finalize (backend, conn) == -1)
> +            return -1;
> +          backend_close (backend, conn);
> +          if (send_newstyle_option_reply (conn, option,
> +                                          NBD_REP_ERR_UNKNOWN) == -1)
> +            return -1;
> +          continue;
> +        }
> 
>          if (send_newstyle_option_reply_info_export (conn, option,
>                                                      NBD_REP_INFO,
> @@ -497,7 +513,7 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
>        }
> 
>        /* Unlike NBD_OPT_EXPORT_NAME, NBD_OPT_GO sends back an ACK
> -       * or ERROR packet.
> +       * or ERROR packet.  If this was NBD_OPT_LIST, call .close.
>         */
>        if (send_newstyle_option_reply (conn, option, NBD_REP_ACK) == -1)
>          return -1;
> diff --git a/server/protocol-handshake-oldstyle.c
b/server/protocol-handshake-oldstyle.c
> index 4efc668b..45a1a486 100644
> --- a/server/protocol-handshake-oldstyle.c
> +++ b/server/protocol-handshake-oldstyle.c
> @@ -52,6 +52,9 @@ protocol_handshake_oldstyle (struct connection *conn)
> 
>    assert (tls != 2); /* Already filtered in main */
> 
> +  /* With oldstyle, our only option if .open or friends fail is to
> +   * disconnect, as we cannot report the problem to the client.
> +   */
>    if (protocol_common_open (conn, &exportsize, &eflags) == -1)
>      return -1;
Yes this seems fine.  Probably best to check that the series doesn't
break ‘make check-valgrind’ before pushing however.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

Richard W.M. Jones

2019-Sep-28 21:38 UTC

head link

Re: [Libguestfs] [nbdkit PATCH v2 5/7] server: Allow longer NBD_OPT

On Fri, Sep 27, 2019 at 11:48:47PM -0500, Eric Blake
wrote:> Fixes the fact that clients could not request the maximum string
> length except with NBD_OPT_EXPORT_LEN.  Updates the testsuite to
> match.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  server/protocol-handshake-newstyle.c | 12 +++++++-----
>  tests/test-long-name.sh              | 10 ++++------
>  2 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/server/protocol-handshake-newstyle.c
b/server/protocol-handshake-newstyle.c
> index 34958360..3b5d144e 100644
> --- a/server/protocol-handshake-newstyle.c
> +++ b/server/protocol-handshake-newstyle.c
> @@ -48,7 +48,7 @@
>  #define MAX_NR_OPTIONS 32
> 
>  /* Maximum length of any option data (bytes). */
> -#define MAX_OPTION_LENGTH 4096
> +#define MAX_OPTION_LENGTH (NBD_MAX_STRING * 4)
> 
>  /* Receive newstyle options. */
>  static int
> @@ -255,7 +255,7 @@ negotiate_handshake_newstyle_options (struct connection
*conn)
>    uint64_t version;
>    uint32_t option;
>    uint32_t optlen;
> -  char data[MAX_OPTION_LENGTH+1];
> +  CLEANUP_FREE char *data = NULL;
Even though you have the CLEANUP here ...
>    struct nbd_export_name_option_reply handshake_finish;
>    const char *optname;
>    uint64_t exportsize;
> @@ -281,6 +281,11 @@ negotiate_handshake_newstyle_options (struct
connection *conn)
>        nbdkit_error ("client option data too long (%" PRIu32
")", optlen);
>        return -1;
>      }
> +    data = malloc (optlen + 1); /* Allowing a trailing NUL helps some uses
*/
> +    if (data == NULL) {
> +      nbdkit_error ("malloc: %m");
> +      return -1;
> +    }
... when I run this patch series under valgrind I get mainly errors
originating at this malloc:

==1251605== 58 bytes in 4 blocks are definitely lost in loss record 4 of 10
==1251605==    at 0x896180B: malloc (vg_replace_malloc.c:309)
==1251605==    by 0x11909F: protocol_handshake_newstyle (protocol-handshake-news
tyle.c:288)
==1251605==    by 0x118804: protocol_handshake (protocol-handshake.c:55)
==1251605==    by 0x112080: handle_single_connection (connections.c:165)
==1251605==    by 0x11B84D: start_thread (sockets.c:276)
==1251605==    by 0x8BB74E1: start_thread (pthread_create.c:479)
==1251605==    by 0x8CD3642: clone (clone.S:95)

I didn't look at it closely but there does appear to be a memory leak
in this patch.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

Possibly Parallel Threads

Search for more apparently analagous threads

Libguestfs - Sep 2019 - [nbdkit PATCH v2 7/7] server: Better newstyle .open failure handling

[Libguestfs] [nbdkit PATCH v2 0/7] Spec compliance patches

[Libguestfs] [nbdkit PATCH v2 1/7] server: Reject -e with too long of a name

[Libguestfs] [nbdkit PATCH v2 2/7] server: Expose -e in $exportname during --run

[Libguestfs] [nbdkit PATCH v2 3/7] server: Switch to fixed-length conn->exportname

[Libguestfs] [nbdkit PATCH v2 4/7] server: Improve check of string validity

[Libguestfs] [nbdkit PATCH v2 5/7] server: Allow longer NBD_OPT

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

[Libguestfs] [nbdkit PATCH v2 7/7] server: Better newstyle .open failure handling

Re: [Libguestfs] [nbdkit PATCH v2 5/7] server: Allow longer NBD_OPT

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

Re: [Libguestfs] [nbdkit PATCH v2 7/7] server: Better newstyle .open failure handling

Re: [Libguestfs] [nbdkit PATCH v2 5/7] server: Allow longer NBD_OPT

Possibly Parallel Threads