search for: check_export_name

...portnamelen; 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;...

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

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

...} + + if (protocol_common_open (exportsize, &conn->eflags, exportname) == -1) return -1; debug ("newstyle negotiation: flags: export 0x%x", conn->eflags); @@ -238,22 +256,13 @@ check_string (uint32_t option, char *buf, uint32_t len, uint32_t maxlen, */ static int check_export_name (uint32_t option, char *buf, - uint32_t exportnamelen, uint32_t maxlen, bool save) + uint32_t exportnamelen, uint32_t maxlen) { GET_CONN; if (check_string (option, buf, exportnamelen, maxlen, "export name") == -1) return -1; - assert (e...

...nection *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 @@ n...

...ocol_common_open (exportsize, &conn->eflags) == -1) return -1; debug ("newstyle negotiation: flags: export 0x%x", conn->eflags); @@ -233,9 +231,11 @@ check_string (uint32_t option, char *buf, uint32_t len, uint32_t maxlen, * validate an export name. */ static int -check_export_name (struct connection *conn, uint32_t option, char *buf, +check_export_name (uint32_t option, char *buf, uint32_t exportnamelen, uint32_t maxlen, bool save) { + struct connection *conn = GET_CONN; + if (check_string (option, buf, exportnamelen, maxlen, "export name")...

v1 was here: https://www.redhat.com/archives/libguestfs/2020-February/msg00081.html v2 replaces struct connection *conn = GET_CONN; with GET_CONN; which sets conn implicitly and asserts that it is non-NULL. If we actually want to test if conn is non-NULL or behave differently, then you must use threadlocal_get_conn() instead, and some existing uses do that. Rich.

The third patch is a large but mechanical change which gets rid of passing around struct connection * entirely within the server, preferring instead to reference the connection through thread-local storage. I hope this is a gateway to simplifying other parts of the code. Rich.