aboutsummaryrefslogtreecommitdiff
path: root/nbd
diff options
context:
space:
mode:
authorEric Blake <eblake@redhat.com>2017-07-17 14:26:35 -0500
committerEric Blake <eblake@redhat.com>2017-07-17 17:06:46 -0500
commit5f66d060dbc37214c9d70305710c3e34c4531d7c (patch)
treedb1d77cf9eb572c8639396f6c4ffc6aeada816cd /nbd
parent48000eb3ecfc91f6588dfafba51d57e7f8f28b7e (diff)
downloadqemu-5f66d060dbc37214c9d70305710c3e34c4531d7c.zip
qemu-5f66d060dbc37214c9d70305710c3e34c4531d7c.tar.gz
qemu-5f66d060dbc37214c9d70305710c3e34c4531d7c.tar.bz2
nbd: Fix server reply to NBD_OPT_EXPORT_NAME of older clients
A typo in commit 23e099c set the size of buf[] used in response to NBD_OPT_EXPORT_NAME according to the length needed for old-style negotiation (4 bytes of flag information) instead of the intended 2 bytes used in new style. If the client doesn't enable NBD_FLAG_C_NO_ZEROES, then the server sends two bytes too many, and is then out of sync in response to the client's next command (the bug is masked when modern qemu is the client, since we enable the no zeroes flag). While touching this code, add some more defines to nbd_internal.h rather than having quite so many magic numbers in the .c; also, use "" initialization rather than memset(), and tweak the oldstyle negotiation to better match the spec description of the layout (since the spec is big-endian, skipping two bytes as 0 followed by writing a 2-byte flag is the same as writing a zero-extended 4-byte flag), to make it a bit easier to follow compared to the spec. [checkpatch.pl has some false positives in the comments] Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20170717192635.17880-3-eblake@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com>
Diffstat (limited to 'nbd')
-rw-r--r--nbd/nbd-internal.h8
-rw-r--r--nbd/server.c18
2 files changed, 16 insertions, 10 deletions
diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index 4065bc6..396ddb4 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -38,9 +38,13 @@
*/
/* Size of all NBD_OPT_*, without payload */
-#define NBD_REQUEST_SIZE (4 + 2 + 2 + 8 + 8 + 4)
+#define NBD_REQUEST_SIZE (4 + 2 + 2 + 8 + 8 + 4)
/* Size of all NBD_REP_* sent in answer to most NBD_OPT_*, without payload */
-#define NBD_REPLY_SIZE (4 + 4 + 8)
+#define NBD_REPLY_SIZE (4 + 4 + 8)
+/* Size of reply to NBD_OPT_EXPORT_NAME */
+#define NBD_REPLY_EXPORT_NAME_SIZE (8 + 2 + 124)
+/* Size of oldstyle negotiation */
+#define NBD_OLDSTYLE_NEGOTIATE_SIZE (8 + 8 + 8 + 4 + 124)
#define NBD_REQUEST_MAGIC 0x25609513
#define NBD_REPLY_MAGIC 0x67446698
diff --git a/nbd/server.c b/nbd/server.c
index 49ed574..82a78bf 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -283,12 +283,16 @@ static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t length,
Error **errp)
{
char name[NBD_MAX_NAME_SIZE + 1];
- char buf[8 + 4 + 124] = "";
+ char buf[NBD_REPLY_EXPORT_NAME_SIZE] = "";
size_t len;
int ret;
/* Client sends:
[20 .. xx] export name (length bytes)
+ Server replies:
+ [ 0 .. 7] size
+ [ 8 .. 9] export flags
+ [10 .. 133] reserved (0) [unless no_zeroes]
*/
trace_nbd_negotiate_handle_export_name();
if (length >= sizeof(name)) {
@@ -800,22 +804,21 @@ static int nbd_negotiate_options(NBDClient *client, uint16_t myflags,
*/
static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
{
- char buf[8 + 8 + 8 + 128];
+ char buf[NBD_OLDSTYLE_NEGOTIATE_SIZE] = "";
int ret;
const uint16_t myflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA |
NBD_FLAG_SEND_WRITE_ZEROES);
bool oldStyle;
- /* Old style negotiation header without options
+ /* Old style negotiation header, no room for options
[ 0 .. 7] passwd ("NBDMAGIC")
[ 8 .. 15] magic (NBD_CLIENT_MAGIC)
[16 .. 23] size
- [24 .. 25] server flags (0)
- [26 .. 27] export flags
+ [24 .. 27] export flags (zero-extended)
[28 .. 151] reserved (0)
- New style negotiation header with options
+ New style negotiation header, client can send options
[ 0 .. 7] passwd ("NBDMAGIC")
[ 8 .. 15] magic (NBD_OPTS_MAGIC)
[16 .. 17] server flags (0)
@@ -825,7 +828,6 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
qio_channel_set_blocking(client->ioc, false, NULL);
trace_nbd_negotiate_begin();
- memset(buf, 0, sizeof(buf));
memcpy(buf, "NBDMAGIC", 8);
oldStyle = client->exp != NULL && !client->tlscreds;
@@ -834,7 +836,7 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
client->exp->nbdflags | myflags);
stq_be_p(buf + 8, NBD_CLIENT_MAGIC);
stq_be_p(buf + 16, client->exp->size);
- stw_be_p(buf + 26, client->exp->nbdflags | myflags);
+ stl_be_p(buf + 24, client->exp->nbdflags | myflags);
if (nbd_write(client->ioc, buf, sizeof(buf), errp) < 0) {
error_prepend(errp, "write failed: ");