search for: exportnamelen

...++++++++++++++++++-- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/server/internal.h b/server/internal.h index da4fae19..0603a747 100644 --- a/server/internal.h +++ b/server/internal.h @@ -182,6 +182,7 @@ struct connection { size_t nr_handles; char *exportname; + 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 785944eb..45a65487 100644 --- a/server/protocol-handshake-newstyle.c +++ b/server/protocol-handshake-newstyle.c @@ -282,6 +282,7 @@ negotiate_ha...

...- 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_expor...

...; 6"); + debug ("newstyle negotiation: %s option length < 6", optname); if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) == -1) @@ -800,7 +803,7 @@ _negotiate_handshake_newstyle_options (struct connection *conn) memcpy (&exportnamelen, &data[0], 4); exportnamelen = be32toh (exportnamelen); if (exportnamelen > optlen-6 /* NB optlen >= 6, see above */) { - debug ("newstyle negotiation: NBD_OPT_GO: export name too long"); + debug ("newstyle negotiation: %s: export name to...

There's no substantial difference over v1, I simply fixed a few whitespace issues, moved one struct around and tidied up the comments. Rich.

The first one is the nastiest - it is an assertion failure caused by a spec-compliant client and introduced by our security fix that was released in 1.14.1. Eric Blake (4): server: Fix regression for NBD_OPT_INFO before NBD_OPT_GO server: Fix back-to-back SET_META_CONTEXT server: Forbid NUL in export and context names server: Fix OPT_GO on different export than SET_META_CONTEXT

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

...bit export length + 16 bit nr info */ + debug ("newstyle negotiation: NBD_OPT_GO option length < 6"); + + if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) + == -1) + return -1; + continue; + } + + { + uint32_t exportnamelen; + uint16_t nrinfos; + uint16_t info; + size_t i; + CLEANUP_FREE char *requested_exportname = NULL; + + /* Validate the name length and number of INFO requests. */ + memcpy (&exportnamelen, &data[0], 4); + exportnamelen = be32toh (exportnamel...

...+++ b/server/connections.c @@ -926,6 +926,90 @@ _negotiate_handshake_newstyle_options (struct connection *conn) conn->structured_replies = true; break; + case NBD_OPT_LIST_META_CONTEXT: + case NBD_OPT_SET_META_CONTEXT: + { + uint32_t opt_index; + uint32_t exportnamelen; + uint32_t nr_queries; + uint32_t querylen; + const char *what; + + optname = name_of_nbd_opt (option); + if (conn_recv_full (conn, data, optlen, "read: %s: %m", optname) == -1) + return -1; + + if (!conn->structured_replies) { +...

...server/internal.h b/server/internal.h index ebc63a52..4911a49b 100644 --- a/server/internal.h +++ b/server/internal.h @@ -246,8 +246,6 @@ struct connection { struct handle *handles; /* One per plugin and filter. */ size_t nr_handles; - char exportname[NBD_MAX_STRING + 1]; - uint32_t exportnamelen; uint32_t cflags; uint16_t eflags; bool handshake_complete; @@ -255,6 +253,9 @@ struct connection { bool structured_replies; bool meta_context_base_allocation; + char *exportname_from_set_meta_context; + char *exportname; + int sockin, sockout; connection_recv_function recv...

This is only lightly tested (against just qemu NBD client), and the code might be structured a little better as the _negotiate_handshake_newstyle_options function has now grown to be huge. Anyway works for me. Rich.

...*conn) continue; } - /* As with NBD_OPT_EXPORT_NAME we print the export name and then - * ignore it. + /* As with NBD_OPT_EXPORT_NAME we print the export name and + * save it in the connection. */ - requested_exportname = malloc (exportnamelen+1); - if (requested_exportname == NULL) { + free (conn->exportname); + conn->exportname = malloc (exportnamelen+1); + if (conn->exportname == NULL) { nbdkit_error ("malloc: %m"); return -1; } - memcpy (requested_ex...

...bit export length + 16 bit nr info */ - debug ("newstyle negotiation: %s option length < 6", optname); - - if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) - == -1) - return -1; - continue; - } - - { - uint32_t exportnamelen; - uint16_t nrinfos; - uint16_t info; - size_t i; - CLEANUP_FREE char *requested_exportname = NULL; - - /* Validate the name length and number of INFO requests. */ - memcpy (&exportnamelen, &data[0], 4); - exportnamelen = be32toh (exportnamel...

These are a couple of patches in preparation for the Block Status implementation. While the patches (especially the second one) are very large they are really just elementary code motion. Rich.

This is the sort of thing I had in mind for option (1) here: https://www.redhat.com/archives/libguestfs/2019-September/msg00047.html It does reveal that the way we currently list exports is naive to say the least ... Rich.

The previous incomplete patch was here: https://www.redhat.com/archives/libguestfs/2019-September/msg00049.html based on earlier discussion here: https://www.redhat.com/archives/libguestfs/2019-September/msg00047.html In v2: - The previous patch was incomplete. This version completes it by adding tests and extending nbdkit-sh-plugin. - nbdkit_export_name now returns NULL for error,

...* Attempt to negotiate structured reads, block status, and NBD_OPT_GO. + Return 1 if haggling completed, 0 if haggling failed but + NBD_OPT_EXPORT_NAME is still viable, or -1 on inability to connect. */ +static int +nbd_newstyle_haggle (struct handle *h) +{ + struct new_option opt; + uint32_t exportnamelen = htobe32 (strlen (export)); + /* For now, we make no NBD_INFO_* requests, relying on the server to + send its defaults. TODO: nbdkit should let plugins report block + sizes, at which point we should request NBD_INFO_BLOCK_SIZE and + obey any sizes set by server. */ + uint16_t nrinfos...

...* other info requests (including NBD_INFO_EXPORT if it was + * requested, because we replied already above). + * XXX NBD_INFO_DESCRIPTION is easy once we add .export_description. */ for (i = 0; i < nrinfos; ++i) { memcpy (&info, &data[4 + exportnamelen + 2 + i*2], 2); info = be16toh (info); switch (info) { case NBD_INFO_EXPORT: /* ignore - reply sent above */ break; + case NBD_INFO_NAME: + { + const char *name = &data[4]; + size_t namelen = exportnamelen; + +...

...chives/libguestfs/2020-July/thread.html#00087 > --- > +++ b/server/internal.h > @@ -246,8 +246,6 @@ struct connection { > struct handle *handles; /* One per plugin and filter. */ > size_t nr_handles; > > - char exportname[NBD_MAX_STRING + 1]; > - uint32_t exportnamelen; > uint32_t cflags; > uint16_t eflags; > bool handshake_complete; > @@ -255,6 +253,9 @@ struct connection { > bool structured_replies; > bool meta_context_base_allocation; > > + char *exportname_from_set_meta_context; > + char *exportname; Intere...

Updated based on other changes that have happened in the meantime: - rely more on cleanup.h (throughout) - split structured read for easier review (patch 2 and 3 were combined in v1) - rely on nbdkit not leaking a server's partial answer (patch 3) - add tests (patch 5) - other bug fixes I found while testing it - drop EOVERFLOW patch for now; it will be separate once upstream NBD protocol

...h->wflags = MSG_MORE; SET_NEXT_STATE (%SEND); return 0; diff --git a/generator/states-newstyle-opt-go.c b/generator/states-newstyle-opt-go.c index eea70cb..06bbaca 100644 --- a/generator/states-newstyle-opt-go.c +++ b/generator/states-newstyle-opt-go.c @@ -26,6 +26,7 @@ htobe32 (/* exportnamelen */ 4 + strlen (h->export_name) + /* nrinfos */ 2); h->wbuf = &h->sbuf; h->wlen = sizeof h->sbuf.option; + h->wflags = MSG_MORE; SET_NEXT_STATE (%SEND); return 0; @@ -38,6 +39,7 @@ h->sbuf.len = htobe32 (exportnamelen); h->wbuf = &h->sbuf;...