From 9d26dfcbab62746b3e66ec7784d75c13ff499669 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 17 Jan 2019 13:36:43 -0600 Subject: nbd/server: Favor [u]int64_t over off_t Although our compile-time environment is set up so that we always support long files with 64-bit off_t, we have no guarantee whether off_t is the same type as int64_t. This requires casts when printing values, and prevents us from directly using qemu_strtoi64() (which will be done in the next patch). Let's just flip to uint64_t where possible, and stick to int64_t for detecting failure of blk_getlength(); we also keep the assertions added in the previous patch that the resulting values fit in 63 bits. The overflow check in nbd_co_receive_request() was already sane (request->from is validated to fit in 63 bits, and request->len is 32 bits, so the addition can't overflow 64 bits), but rewrite it in a form easier to recognize as a typical overflow check. Rename the variable 'description' to keep line lengths reasonable. Suggested-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Eric Blake Message-Id: <20190117193658.16413-7-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy --- include/block/nbd.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/block/nbd.h b/include/block/nbd.h index 1971b55789..24be9570bb 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -294,8 +294,8 @@ int nbd_errno_to_system_errno(int err); typedef struct NBDExport NBDExport; typedef struct NBDClient NBDClient; -NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset, off_t size, - const char *name, const char *description, +NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset, + uint64_t size, const char *name, const char *desc, const char *bitmap, uint16_t nbdflags, void (*close)(NBDExport *), bool writethrough, BlockBackend *on_eject_blk, Error **errp); -- cgit v1.2.3-55-g7522 From 6dc1667d6881add34e9bad48ac2a848134ea8a6d Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 17 Jan 2019 13:36:46 -0600 Subject: nbd/client: Move export name into NBDExportInfo Refactor the 'name' parameter of nbd_receive_negotiate() from being a separate parameter into being part of the in-out 'info'. This also spills over to a simplification of nbd_opt_go(). The main driver for this refactoring is that an upcoming patch would like to add support to qemu-nbd to list information about all exports available on a server, where the name(s) will be provided by the server instead of the client. But another benefit is that we can now allow the client to explicitly specify the empty export name "" even when connecting to an oldstyle server (even if qemu is no longer such a server after commit 7f7dfe2a). Signed-off-by: Eric Blake Reviewed-by: Richard W.M. Jones Reviewed-by: Vladimir Sementsov-Ogievskiy Message-Id: <20190117193658.16413-10-eblake@redhat.com> --- block/nbd-client.c | 5 +++-- include/block/nbd.h | 8 ++++---- nbd/client.c | 39 ++++++++++++++++++--------------------- nbd/trace-events | 2 +- qemu-nbd.c | 6 ++++-- 5 files changed, 30 insertions(+), 30 deletions(-) (limited to 'include') diff --git a/block/nbd-client.c b/block/nbd-client.c index ef32075971..3309376bc1 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -999,10 +999,11 @@ int nbd_client_init(BlockDriverState *bs, client->info.structured_reply = true; client->info.base_allocation = true; client->info.x_dirty_bitmap = g_strdup(x_dirty_bitmap); - ret = nbd_receive_negotiate(QIO_CHANNEL(sioc), export, - tlscreds, hostname, + client->info.name = g_strdup(export ?: ""); + ret = nbd_receive_negotiate(QIO_CHANNEL(sioc), tlscreds, hostname, &client->ioc, &client->info, errp); g_free(client->info.x_dirty_bitmap); + g_free(client->info.name); if (ret < 0) { logout("Failed to negotiate with the NBD server\n"); return ret; diff --git a/include/block/nbd.h b/include/block/nbd.h index 24be9570bb..00d3eb57d9 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -262,6 +262,7 @@ struct NBDExportInfo { /* Set by client before nbd_receive_negotiate() */ bool request_sizes; char *x_dirty_bitmap; + char *name; /* must be non-NULL */ /* In-out fields, set by client before nbd_receive_negotiate() and * updated by server results during nbd_receive_negotiate() */ @@ -279,10 +280,9 @@ struct NBDExportInfo { }; typedef struct NBDExportInfo NBDExportInfo; -int nbd_receive_negotiate(QIOChannel *ioc, const char *name, - QCryptoTLSCreds *tlscreds, const char *hostname, - QIOChannel **outioc, NBDExportInfo *info, - Error **errp); +int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds, + const char *hostname, QIOChannel **outioc, + 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 fd4ba8dec3..8227e69478 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -330,15 +330,14 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, char **description, } -/* Returns -1 if NBD_OPT_GO proves the export @wantname cannot be +/* Returns -1 if NBD_OPT_GO proves the export @info->name cannot be * used, 0 if NBD_OPT_GO is unsupported (fall back to NBD_OPT_LIST and * NBD_OPT_EXPORT_NAME in that case), and > 0 if the export is good to - * go (with @info populated). */ -static int nbd_opt_go(QIOChannel *ioc, const char *wantname, - NBDExportInfo *info, Error **errp) + * go (with the rest of @info populated). */ +static int nbd_opt_go(QIOChannel *ioc, NBDExportInfo *info, Error **errp) { NBDOptionReply reply; - uint32_t len = strlen(wantname); + uint32_t len = strlen(info->name); uint16_t type; int error; char *buf; @@ -348,10 +347,10 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname, * flags still 0 is a witness of a broken server. */ info->flags = 0; - trace_nbd_opt_go_start(wantname); + trace_nbd_opt_go_start(info->name); buf = g_malloc(4 + len + 2 + 2 * info->request_sizes + 1); stl_be_p(buf, len); - memcpy(buf + 4, wantname, len); + memcpy(buf + 4, info->name, len); /* At most one request, everything else up to server */ stw_be_p(buf + 4 + len, info->request_sizes); if (info->request_sizes) { @@ -753,10 +752,9 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc, return 0; } -int nbd_receive_negotiate(QIOChannel *ioc, const char *name, - QCryptoTLSCreds *tlscreds, const char *hostname, - QIOChannel **outioc, NBDExportInfo *info, - Error **errp) +int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds, + const char *hostname, QIOChannel **outioc, + NBDExportInfo *info, Error **errp) { uint64_t magic; int rc; @@ -766,6 +764,8 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, trace_nbd_receive_negotiate(tlscreds, hostname ? hostname : ""); + assert(info->name); + trace_nbd_receive_negotiate_name(info->name); info->structured_reply = false; info->base_allocation = false; rc = -EINVAL; @@ -834,10 +834,6 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, goto fail; } } - if (!name) { - trace_nbd_receive_negotiate_default_name(); - name = ""; - } if (fixedNewStyle) { int result; @@ -853,7 +849,8 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, if (info->structured_reply && base_allocation) { result = nbd_negotiate_simple_meta_context( - ioc, name, info->x_dirty_bitmap ?: "base:allocation", + ioc, info->name, + info->x_dirty_bitmap ?: "base:allocation", &info->meta_base_allocation_id, errp); if (result < 0) { goto fail; @@ -866,7 +863,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, * TLS). If it is not available, fall back to * NBD_OPT_LIST for nicer error messages about a missing * export, then use NBD_OPT_EXPORT_NAME. */ - result = nbd_opt_go(ioc, name, info, errp); + result = nbd_opt_go(ioc, info, errp); if (result < 0) { goto fail; } @@ -879,12 +876,12 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, * query gives us better error reporting if the * export name is not available. */ - if (nbd_receive_query_exports(ioc, name, errp) < 0) { + if (nbd_receive_query_exports(ioc, info->name, errp) < 0) { goto fail; } } /* write the export name request */ - if (nbd_send_option_request(ioc, NBD_OPT_EXPORT_NAME, -1, name, + if (nbd_send_option_request(ioc, NBD_OPT_EXPORT_NAME, -1, info->name, errp) < 0) { goto fail; } @@ -904,8 +901,8 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, } else if (magic == NBD_CLIENT_MAGIC) { uint32_t oldflags; - if (name) { - error_setg(errp, "Server does not support export names"); + if (*info->name) { + error_setg(errp, "Server does not support non-empty export names"); goto fail; } if (tlscreds) { diff --git a/nbd/trace-events b/nbd/trace-events index d1e1ca64ee..c3966d2b65 100644 --- a/nbd/trace-events +++ b/nbd/trace-events @@ -17,7 +17,7 @@ nbd_opt_meta_reply(const char *context, uint32_t id) "Received mapping of contex nbd_receive_negotiate(void *tlscreds, const char *hostname) "Receiving negotiation tlscreds=%p hostname=%s" nbd_receive_negotiate_magic(uint64_t magic) "Magic is 0x%" PRIx64 nbd_receive_negotiate_server_flags(uint32_t globalflags) "Global flags are 0x%" PRIx32 -nbd_receive_negotiate_default_name(void) "Using default NBD export name \"\"" +nbd_receive_negotiate_name(const char *name) "Requesting NBD export name '%s'" nbd_receive_negotiate_size_flags(uint64_t size, uint16_t flags) "Size is %" PRIu64 ", export flags 0x%" PRIx16 nbd_init_set_socket(void) "Setting NBD socket" nbd_init_set_block_size(unsigned long block_size) "Setting block size to %lu" diff --git a/qemu-nbd.c b/qemu-nbd.c index efca0e44a9..3c538700ea 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -264,7 +264,7 @@ static void *show_parts(void *arg) static void *nbd_client_thread(void *arg) { char *device = arg; - NBDExportInfo info = { .request_sizes = false, }; + NBDExportInfo info = { .request_sizes = false, .name = g_strdup("") }; QIOChannelSocket *sioc; int fd; int ret; @@ -279,7 +279,7 @@ static void *nbd_client_thread(void *arg) goto out; } - ret = nbd_receive_negotiate(QIO_CHANNEL(sioc), NULL, + ret = nbd_receive_negotiate(QIO_CHANNEL(sioc), NULL, NULL, NULL, &info, &local_error); if (ret < 0) { if (local_error) { @@ -318,6 +318,7 @@ static void *nbd_client_thread(void *arg) } close(fd); object_unref(OBJECT(sioc)); + g_free(info.name); kill(getpid(), SIGTERM); return (void *) EXIT_SUCCESS; @@ -326,6 +327,7 @@ out_fd: out_socket: object_unref(OBJECT(sioc)); out: + g_free(info.name); kill(getpid(), SIGTERM); return (void *) EXIT_FAILURE; } -- cgit v1.2.3-55-g7522 From 2df94eb52b68d16f8a050bc28dd94a8c7d3366ec Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 17 Jan 2019 13:36:47 -0600 Subject: nbd/client: Change signature of nbd_negotiate_simple_meta_context() Pass 'info' instead of three separate parameters related to info, when requesting the server to set the meta context. Update the NBDExportInfo struct to rename the received id field to match the fact that we are currently overloading the field to match whatever context the user supplied through the x-dirty-bitmap hack, as well as adding a TODO comment to remind future patches about a desire to request two contexts at once. Signed-off-by: Eric Blake Reviewed-by: Richard W.M. Jones Reviewed-by: Vladimir Sementsov-Ogievskiy Message-Id: <20190117193658.16413-11-eblake@redhat.com> --- block/nbd-client.c | 4 ++-- include/block/nbd.h | 2 +- nbd/client.c | 53 +++++++++++++++++++++++++---------------------------- 3 files changed, 28 insertions(+), 31 deletions(-) (limited to 'include') diff --git a/block/nbd-client.c b/block/nbd-client.c index 3309376bc1..813539676d 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -249,11 +249,11 @@ static int nbd_parse_blockstatus_payload(NBDClientSession *client, } context_id = payload_advance32(&payload); - if (client->info.meta_base_allocation_id != context_id) { + if (client->info.context_id != context_id) { error_setg(errp, "Protocol error: unexpected context id %d for " "NBD_REPLY_TYPE_BLOCK_STATUS, when negotiated context " "id is %d", context_id, - client->info.meta_base_allocation_id); + client->info.context_id); return -EINVAL; } diff --git a/include/block/nbd.h b/include/block/nbd.h index 00d3eb57d9..be19aac2fc 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -276,7 +276,7 @@ struct NBDExportInfo { uint32_t opt_block; uint32_t max_block; - uint32_t meta_base_allocation_id; + uint32_t context_id; }; typedef struct NBDExportInfo NBDExportInfo; diff --git a/nbd/client.c b/nbd/client.c index 8227e69478..77993890f0 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -630,26 +630,30 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc, } /* nbd_negotiate_simple_meta_context: - * Set one meta context. Simple means that reply must contain zero (not - * negotiated) or one (negotiated) contexts. More contexts would be considered - * as a protocol error. It's also implied that meta-data query equals queried - * context name, so, if server replies with something different than @context, - * it is considered an error too. - * return 1 for successful negotiation, context_id is set + * Request the server to set the meta context for export @info->name + * using @info->x_dirty_bitmap with a fallback to "base:allocation", + * setting @info->context_id to the resulting id. Fail if the server + * responds with more than one context or with a context different + * than the query. + * return 1 for successful negotiation, * 0 if operation is unsupported, * -1 with errp set for any other error */ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc, - const char *export, - const char *context, - uint32_t *context_id, + NBDExportInfo *info, Error **errp) { + /* + * TODO: Removing the x_dirty_bitmap hack will mean refactoring + * this function to request and store ids for multiple contexts + * (both base:allocation and a dirty bitmap), at which point this + * function should lose the term _simple. + */ int ret; NBDOptionReply reply; - uint32_t received_id = 0; + const char *context = info->x_dirty_bitmap ?: "base:allocation"; bool received = false; - uint32_t export_len = strlen(export); + uint32_t export_len = strlen(info->name); uint32_t context_len = strlen(context); uint32_t data_len = sizeof(export_len) + export_len + sizeof(uint32_t) + /* number of queries */ @@ -657,9 +661,9 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc, char *data = g_malloc(data_len); char *p = data; - trace_nbd_opt_meta_request(context, export); + trace_nbd_opt_meta_request(context, info->name); stl_be_p(p, export_len); - memcpy(p += sizeof(export_len), export, export_len); + memcpy(p += sizeof(export_len), info->name, export_len); stl_be_p(p += export_len, 1); stl_be_p(p += sizeof(uint32_t), context_len); memcpy(p += sizeof(context_len), context, context_len); @@ -685,7 +689,7 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc, if (reply.type == NBD_REP_META_CONTEXT) { char *name; - if (reply.length != sizeof(received_id) + context_len) { + if (reply.length != sizeof(info->context_id) + context_len) { error_setg(errp, "Failed to negotiate meta context '%s', server " "answered with unexpected length %" PRIu32, context, reply.length); @@ -693,12 +697,13 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc, return -1; } - if (nbd_read(ioc, &received_id, sizeof(received_id), errp) < 0) { + if (nbd_read(ioc, &info->context_id, sizeof(info->context_id), + errp) < 0) { return -1; } - received_id = be32_to_cpu(received_id); + info->context_id = be32_to_cpu(info->context_id); - reply.length -= sizeof(received_id); + reply.length -= sizeof(info->context_id); name = g_malloc(reply.length + 1); if (nbd_read(ioc, name, reply.length, errp) < 0) { g_free(name); @@ -715,7 +720,7 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc, } g_free(name); - trace_nbd_opt_meta_reply(context, received_id); + trace_nbd_opt_meta_reply(context, info->context_id); received = true; /* receive NBD_REP_ACK */ @@ -744,12 +749,7 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc, return -1; } - if (received) { - *context_id = received_id; - return 1; - } - - return 0; + return received; } int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds, @@ -848,10 +848,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds, } if (info->structured_reply && base_allocation) { - result = nbd_negotiate_simple_meta_context( - ioc, info->name, - info->x_dirty_bitmap ?: "base:allocation", - &info->meta_base_allocation_id, errp); + result = nbd_negotiate_simple_meta_context(ioc, info, errp); if (result < 0) { goto fail; } -- cgit v1.2.3-55-g7522 From d21a2d3451d7f1defea5104ce28938f228fab0d4 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 17 Jan 2019 13:36:54 -0600 Subject: nbd/client: Add nbd_receive_export_list() 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 Reviewed-by: Richard W.M. Jones Message-Id: <20190117193658.16413-18-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy --- include/block/nbd.h | 15 +++++- nbd/client.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 143 insertions(+), 4 deletions(-) (limited to 'include') diff --git a/include/block/nbd.h b/include/block/nbd.h index be19aac2fc..19332b4671 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 * * 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 fa1657a1cb..8a32169970 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 : ""); - *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) -- cgit v1.2.3-55-g7522 From 0b576b6bfb56291bb13db0a54d99adf2f3706030 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 17 Jan 2019 13:36:55 -0600 Subject: nbd/client: Add meta contexts to nbd_receive_export_list() 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 continues the work of the previous patch, by adding the ability to track the list of available meta contexts into NBDExportInfo. It benefits from the recent refactoring patches with a new nbd_list_meta_contexts() that reuses much of the same framework as setting a meta context. Note: a malicious server could exhaust memory of a client by feeding an unending loop of contexts; perhaps we could place a limit on how many we are willing to receive. But this is no different from our earlier analysis on a server sending an unending list of exports, and the death of a client due to memory exhaustion when the client was going to exit soon anyways is not really a denial of service attack. Signed-off-by: Eric Blake Reviewed-by: Richard W.M. Jones Reviewed-by: Vladimir Sementsov-Ogievskiy Message-Id: <20190117193658.16413-19-eblake@redhat.com> --- include/block/nbd.h | 2 ++ nbd/client.c | 41 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 41 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/block/nbd.h b/include/block/nbd.h index 19332b4671..4faf394e34 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -284,6 +284,8 @@ struct NBDExportInfo { /* Set by server results during nbd_receive_export_list() */ char *description; + int n_contexts; + char **contexts; }; typedef struct NBDExportInfo NBDExportInfo; diff --git a/nbd/client.c b/nbd/client.c index 8a32169970..798b82f205 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -817,6 +817,36 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc, return received; } +/* + * nbd_list_meta_contexts: + * Request the server to list all meta contexts for export @info->name. + * return 0 if list is complete (even if empty), + * -1 with errp set for any error + */ +static int nbd_list_meta_contexts(QIOChannel *ioc, + NBDExportInfo *info, + Error **errp) +{ + int ret; + + if (nbd_send_meta_query(ioc, NBD_OPT_LIST_META_CONTEXT, + info->name, NULL, errp) < 0) { + return -1; + } + + while (1) { + char *context; + + ret = nbd_receive_one_meta_context(ioc, NBD_OPT_LIST_META_CONTEXT, + &context, NULL, errp); + if (ret <= 0) { + return ret; + } + info->contexts = g_renew(char *, info->contexts, ++info->n_contexts); + info->contexts[info->n_contexts - 1] = context; + } +} + /* * nbd_start_negotiate: * Start the handshake to the server. After a positive return, the server @@ -1066,7 +1096,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds, /* Clean up result of nbd_receive_export_list */ void nbd_free_export_list(NBDExportInfo *info, int count) { - int i; + int i, j; if (!info) { return; @@ -1075,6 +1105,10 @@ void nbd_free_export_list(NBDExportInfo *info, int count) for (i = 0; i < count; i++) { g_free(info[i].name); g_free(info[i].description); + for (j = 0; j < info[i].n_contexts; j++) { + g_free(info[i].contexts[j]); + } + g_free(info[i].contexts); } g_free(info); } @@ -1144,7 +1178,10 @@ int nbd_receive_export_list(QIOChannel *ioc, QCryptoTLSCreds *tlscreds, break; } - /* TODO: Grab meta contexts */ + if (result == 3 && + nbd_list_meta_contexts(ioc, &array[i], errp) < 0) { + goto out; + } } /* Send NBD_OPT_ABORT as a courtesy before hanging up */ -- cgit v1.2.3-55-g7522