OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

nbd: Fix server reply to NBD_OPT_EXPORT_NAME of older clients

Browse Source 
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>
This commit is contained in:
Eric Blake 2017-07-17 14:26:35 -05:00
parent 48000eb3ec
commit 5f66d060db
2 changed files with 16 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
nbd/nbd-internal.h
View File

@ -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

18
nbd/server.c
View File

@ -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: ");