10:01 p.m.
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.
I also don't see the point in hacking up libnbd to become such a
client.
Fixes: 4f303e44
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
server/internal.h | 1 +
server/protocol-handshake-newstyle.c | 24 ++++++++++++++++++++++--
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_handshake_newstyle_options (struct connection *conn)
nbdkit_error ("strndup: %m");
return -1;
}
+ conn->exportnamelen = optlen;
if (strlen (conn->exportname) != optlen) {
nbdkit_error ("export name must not contain NUL bytes");
return -1;
@@ -420,14 +421,21 @@ 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. For NBD_OPT_GO, 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 (option == NBD_OPT_GO && conn->meta_context_base_allocation
&&
+ (exportnamelen != conn->exportnamelen ||
+ memcmp (conn->exportname, &data[4], exportnamelen) != 0))
+ conn->meta_context_base_allocation = false;
free (conn->exportname);
conn->exportname = strndup (&data[4], exportnamelen);
if (conn->exportname == NULL) {
nbdkit_error ("malloc: %m");
return -1;
}
+ conn->exportnamelen = exportnamelen;
if (strlen (conn->exportname) != exportnamelen) {
nbdkit_error ("export name must not contain NUL bytes");
return -1;
@@ -552,7 +560,10 @@ negotiate_handshake_newstyle_options (struct connection *conn)
continue;
}
- /* Discard the export name, after checking that it is valid. */
+ /* 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 = "checking export name";
@@ -561,6 +572,15 @@ negotiate_handshake_newstyle_options (struct connection *conn)
what = "export name must not contain NUL bytes";
if (strnlen (&data[4], exportnamelen) != exportnamelen)
goto opt_meta_invalid_option_len;
+ if (option == NBD_OPT_SET_META_CONTEXT) {
+ free (conn->exportname);
+ conn->exportname = strndup (&data[4], exportnamelen);
+ if (conn->exportname == NULL) {
+ nbdkit_error ("strndup: %m");
+ return -1;
+ }
+ conn->exportnamelen = exportnamelen;
+ }
opt_index = 4 + exportnamelen;
/* Read the number of queries. */
--
2.21.0