qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: nsoffer@redhat.com, rjones@redhat.com, jsnow@redhat.com,
	vsementsov@virtuozzo.com, qemu-block@nongnu.org,
	Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
Subject: [Qemu-devel] [PATCH v4 17/21] nbd/client: Add nbd_receive_export_list()
Date: Thu, 17 Jan 2019 13:36:54 -0600	[thread overview]
Message-ID: <20190117193658.16413-18-eblake@redhat.com> (raw)
In-Reply-To: <20190117193658.16413-1-eblake@redhat.com>

We want to be able to detect whether a given qemu NBD server is
exposing the right export(s) and dirty bitmaps, at least for
regression testing.  We could use 'nbd-client -l' from the upstream
NBD project to list exports, but it's annoying to rely on
out-of-tree binaries; furthermore, nbd-client doesn't necessarily
know about all of the qemu NBD extensions.  Thus, we plan on adding
a new mode to qemu-nbd that merely sniffs all possible information
from the server during handshake phase, then disconnects and dumps
the information.

This patch adds the low-level client code for grabbing the list
of exports.  It benefits from the recent refactoring patches, in
order to share as much code as possible when it comes to doing
validation of server replies.  The resulting information is stored
in an array of NBDExportInfo which has been expanded to any
description string, along with a convenience function for freeing
the list.

Note: a malicious server could exhaust memory of a client by feeding
an unending loop of exports; perhaps we should place a limit on how
many we are willing to receive. But note that a server could
reasonably be serving an export for every file in a large directory,
where an arbitrary limit in the client means we can't list anything
from such a server; the same happens if we just run until the client
fails to malloc() and thus dies by an abort(), where the limit is
no longer arbitrary but determined by available memory.  Since the
client is already planning on being short-lived, it's hard to call
this a denial of service attack that would starve off other uses,
so it does not appear to be a security issue.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

---
v4: split out nbd_opt_info_or_go tweaks [Vladimir]
v3: mention security (non-)issue in commit message [Rich], formatting
tweaks
---
 include/block/nbd.h |  15 ++++-
 nbd/client.c        | 132 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 143 insertions(+), 4 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index be19aac2fc7..19332b46715 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -1,5 +1,5 @@
 /*
- *  Copyright (C) 2016-2017 Red Hat, Inc.
+ *  Copyright (C) 2016-2019 Red Hat, Inc.
  *  Copyright (C) 2005  Anthony Liguori <anthony@codemonkey.ws>
  *
  *  Network Block Device
@@ -262,6 +262,9 @@ struct NBDExportInfo {
     /* Set by client before nbd_receive_negotiate() */
     bool request_sizes;
     char *x_dirty_bitmap;
+
+    /* Set by client before nbd_receive_negotiate(), or by server results
+     * during nbd_receive_export_list() */
     char *name; /* must be non-NULL */

     /* In-out fields, set by client before nbd_receive_negotiate() and
@@ -269,7 +272,8 @@ struct NBDExportInfo {
     bool structured_reply;
     bool base_allocation; /* base:allocation context for NBD_CMD_BLOCK_STATUS */

-    /* Set by server results during nbd_receive_negotiate() */
+    /* Set by server results during nbd_receive_negotiate() and
+     * nbd_receive_export_list() */
     uint64_t size;
     uint16_t flags;
     uint32_t min_block;
@@ -277,12 +281,19 @@ struct NBDExportInfo {
     uint32_t max_block;

     uint32_t context_id;
+
+    /* Set by server results during nbd_receive_export_list() */
+    char *description;
 };
 typedef struct NBDExportInfo NBDExportInfo;

 int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
                           const char *hostname, QIOChannel **outioc,
                           NBDExportInfo *info, Error **errp);
+void nbd_free_export_list(NBDExportInfo *info, int count);
+int nbd_receive_export_list(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
+                            const char *hostname, NBDExportInfo **info,
+                            Error **errp);
 int nbd_init(int fd, QIOChannelSocket *sioc, NBDExportInfo *info,
              Error **errp);
 int nbd_send_request(QIOChannel *ioc, NBDRequest *request);
diff --git a/nbd/client.c b/nbd/client.c
index fa1657a1cb3..8a32169970d 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -836,7 +836,9 @@ static int nbd_start_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,

     trace_nbd_start_negotiate(tlscreds, hostname ? hostname : "<null>");

-    *zeroes = true;
+    if (zeroes) {
+        *zeroes = true;
+    }
     if (outioc) {
         *outioc = NULL;
     }
@@ -880,7 +882,9 @@ static int nbd_start_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
             clientflags |= NBD_FLAG_C_FIXED_NEWSTYLE;
         }
         if (globalflags & NBD_FLAG_NO_ZEROES) {
-            *zeroes = false;
+            if (zeroes) {
+                *zeroes = false;
+            }
             clientflags |= NBD_FLAG_C_NO_ZEROES;
         }
         /* client requested flags */
@@ -1059,6 +1063,130 @@ int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
     return 0;
 }

+/* Clean up result of nbd_receive_export_list */
+void nbd_free_export_list(NBDExportInfo *info, int count)
+{
+    int i;
+
+    if (!info) {
+        return;
+    }
+
+    for (i = 0; i < count; i++) {
+        g_free(info[i].name);
+        g_free(info[i].description);
+    }
+    g_free(info);
+}
+
+/*
+ * nbd_receive_export_list:
+ * Query details about a server's exports, then disconnect without
+ * going into transmission phase. Return a count of the exports listed
+ * in @info by the server, or -1 on error. Caller must free @info using
+ * nbd_free_export_list().
+ */
+int nbd_receive_export_list(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
+                            const char *hostname, NBDExportInfo **info,
+                            Error **errp)
+{
+    int result;
+    int count = 0;
+    int i;
+    int rc;
+    int ret = -1;
+    NBDExportInfo *array = NULL;
+    QIOChannel *sioc = NULL;
+
+    *info = NULL;
+    result = nbd_start_negotiate(ioc, tlscreds, hostname, &sioc, true, NULL,
+                                 errp);
+    if (tlscreds && sioc) {
+        ioc = sioc;
+    }
+
+    switch (result) {
+    case 2:
+    case 3:
+        /* newstyle - use NBD_OPT_LIST to populate array, then try
+         * NBD_OPT_INFO on each array member. If structured replies
+         * are enabled, also try NBD_OPT_LIST_META_CONTEXT. */
+        if (nbd_send_option_request(ioc, NBD_OPT_LIST, 0, NULL, errp) < 0) {
+            goto out;
+        }
+        while (1) {
+            char *name;
+            char *desc;
+
+            rc = nbd_receive_list(ioc, &name, &desc, errp);
+            if (rc < 0) {
+                goto out;
+            } else if (rc == 0) {
+                break;
+            }
+            array = g_renew(NBDExportInfo, array, ++count);
+            memset(&array[count - 1], 0, sizeof(*array));
+            array[count - 1].name = name;
+            array[count - 1].description = desc;
+            array[count - 1].structured_reply = result == 3;
+        }
+
+        for (i = 0; i < count; i++) {
+            array[i].request_sizes = true;
+            rc = nbd_opt_info_or_go(ioc, NBD_OPT_INFO, &array[i], errp);
+            if (rc < 0) {
+                goto out;
+            } else if (rc == 0) {
+                /*
+                 * Pointless to try rest of loop. If OPT_INFO doesn't work,
+                 * it's unlikely that meta contexts work either
+                 */
+                break;
+            }
+
+            /* TODO: Grab meta contexts */
+        }
+
+        /* Send NBD_OPT_ABORT as a courtesy before hanging up */
+        nbd_send_opt_abort(ioc);
+        break;
+    case 1: /* newstyle, but limited to EXPORT_NAME */
+        error_setg(errp, "Server does not support export lists");
+        /* We can't even send NBD_OPT_ABORT, so merely hang up */
+        goto out;
+    case 0: /* oldstyle, parse length and flags */
+        array = g_new0(NBDExportInfo, 1);
+        array->name = g_strdup("");
+        count = 1;
+
+        if (nbd_negotiate_finish_oldstyle(ioc, array, errp) < 0) {
+            goto out;
+        }
+
+        /* Send NBD_CMD_DISC as a courtesy to the server, but ignore all
+         * errors now that we have the information we wanted. */
+        if (nbd_drop(ioc, 124, NULL) == 0) {
+            NBDRequest request = { .type = NBD_CMD_DISC };
+
+            nbd_send_request(ioc, &request);
+        }
+        break;
+    default:
+        goto out;
+    }
+
+    *info = array;
+    array = NULL;
+    ret = count;
+
+ out:
+    qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
+    qio_channel_close(ioc, NULL);
+    object_unref(OBJECT(sioc));
+    nbd_free_export_list(array, count);
+    return ret;
+}
+
 #ifdef __linux__
 int nbd_init(int fd, QIOChannelSocket *sioc, NBDExportInfo *info,
              Error **errp)
-- 
2.20.1

  parent reply	other threads:[~2019-01-17 19:38 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-17 19:36 [Qemu-devel] [PATCH v4 00/21] nbd: add qemu-nbd --list Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 01/21] iotests: Make 233 output more reliable Eric Blake
2019-01-18  8:28   ` Vladimir Sementsov-Ogievskiy
2019-01-18 10:04     ` Daniel P. Berrangé
2019-01-18 10:02   ` Daniel P. Berrangé
2019-01-18 15:56     ` Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 02/21] maint: Allow for EXAMPLES in texi2pod Eric Blake
2019-01-18 12:41   ` Vladimir Sementsov-Ogievskiy
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 03/21] qemu-nbd: Enhance man page Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 04/21] qemu-nbd: Sanity check partition bounds Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 05/21] nbd/server: Hoist length check to qmp_nbd_server_add Eric Blake
2019-01-18  9:48   ` Vladimir Sementsov-Ogievskiy
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 06/21] nbd/server: Favor [u]int64_t over off_t Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 07/21] qemu-nbd: Avoid strtol open-coding Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 08/21] nbd/client: Refactor nbd_receive_list() Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 09/21] nbd/client: Move export name into NBDExportInfo Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 10/21] nbd/client: Change signature of nbd_negotiate_simple_meta_context() Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 11/21] nbd/client: Split out nbd_send_meta_query() Eric Blake
2019-01-18  9:59   ` Vladimir Sementsov-Ogievskiy
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 12/21] nbd/client: Split out nbd_receive_one_meta_context() Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 13/21] nbd/client: Refactor return of nbd_receive_negotiate() Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 14/21] nbd/client: Split handshake into two functions Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 15/21] nbd/client: Pull out oldstyle size determination Eric Blake
2019-01-18 10:04   ` Vladimir Sementsov-Ogievskiy
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 16/21] nbd/client: Refactor nbd_opt_go() to support NBD_OPT_INFO Eric Blake
2019-01-18 11:54   ` Vladimir Sementsov-Ogievskiy
2019-01-17 19:36 ` Eric Blake [this message]
2019-01-18 12:07   ` [Qemu-devel] [PATCH v4 17/21] nbd/client: Add nbd_receive_export_list() Vladimir Sementsov-Ogievskiy
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 18/21] nbd/client: Add meta contexts to nbd_receive_export_list() Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 19/21] qemu-nbd: Add --list option Eric Blake
2019-01-18 12:28   ` Vladimir Sementsov-Ogievskiy
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 20/21] nbd/client: Work around 3.0 bug for listing meta contexts Eric Blake
2019-01-17 19:36 ` [Qemu-devel] [PATCH v4 21/21] iotests: Enhance 223, 233 to cover 'qemu-nbd --list' Eric Blake
2019-01-18  8:15 ` [Qemu-devel] [PATCH v4 00/21] nbd: add qemu-nbd --list Vladimir Sementsov-Ogievskiy
2019-01-18 13:47   ` Vladimir Sementsov-Ogievskiy
2019-01-18 16:06     ` Eric Blake
2019-01-18 22:47     ` Eric Blake
2019-01-19  8:07       ` Richard W.M. Jones
2019-01-19 11:39       ` Richard W.M. Jones
2019-01-20 17:42         ` Richard W.M. Jones
2019-01-18 16:08 ` Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190117193658.16413-18-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=nsoffer@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rjones@redhat.com \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).