mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

nbd/client: Correctly handle bad server REP_META_CONTEXT

Browse Source 
It's never a good idea to blindly read for size bytes as
returned by the server without first validating that the size
is within bounds; a malicious or buggy server could cause us
to hang or get out of sync from reading further messages.

It may be smarter to try and teach the client to cope with
unexpected context ids by silently ignoring them instead of
hanging up on the server, but for now, if the server doesn't
reply with exactly the one context we expect, it's easier to
just give up - however, if we give up for any reason other
than an I/O failure, we might as well try to politely tell
the server we are quitting rather than continuing.

Fix some typos in the process.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180329231837.1914680-1-eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
This commit is contained in:
Eric Blake 2018-03-29 18:18:37 -05:00
parent 00d96a4612
commit 260e34dbb7
1 changed files with 21 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
nbd/client.c
View File

@ -599,8 +599,8 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc,
* Set one meta context. Simple means that reply must contain zero (not * Set one meta context. Simple means that reply must contain zero (not
* negotiated) or one (negotiated) contexts. More contexts would be considered * negotiated) or one (negotiated) contexts. More contexts would be considered
* as a protocol error. It's also implied that meta-data query equals queried * as a protocol error. It's also implied that meta-data query equals queried
* context name, so, if server replies with something different then @context, * context name, so, if server replies with something different than @context,
* it considered as error too. * it is considered an error too.
* return 1 for successful negotiation, context_id is set * return 1 for successful negotiation, context_id is set
* 0 if operation is unsupported, * 0 if operation is unsupported,
* -1 with errp set for any other error * -1 with errp set for any other error
@ -649,25 +649,33 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc,
if (reply.type == NBD_REP_META_CONTEXT) { if (reply.type == NBD_REP_META_CONTEXT) {
char *name; char *name;
size_t len;
if (reply.length != sizeof(received_id) + context_len) {
error_setg(errp, "Failed to negotiate meta context '%s', server "
"answered with unexpected length %" PRIu32, context,
reply.length);
nbd_send_opt_abort(ioc);
return -1;
}
if (nbd_read(ioc, &received_id, sizeof(received_id), errp) < 0) { if (nbd_read(ioc, &received_id, sizeof(received_id), errp) < 0) {
return -1; return -1;
} }
be32_to_cpus(&received_id); be32_to_cpus(&received_id);
len = reply.length - sizeof(received_id); reply.length -= sizeof(received_id);
name = g_malloc(len + 1); name = g_malloc(reply.length + 1);
if (nbd_read(ioc, name, len, errp) < 0) { if (nbd_read(ioc, name, reply.length, errp) < 0) {
g_free(name); g_free(name);
return -1; return -1;
} }
name[len] = '\0'; name[reply.length] = '\0';
if (strcmp(context, name)) { if (strcmp(context, name)) {
error_setg(errp, "Failed to negotiate meta context '%s', server " error_setg(errp, "Failed to negotiate meta context '%s', server "
"answered with different context '%s'", context, "answered with different context '%s'", context,
name); name);
g_free(name); g_free(name);
nbd_send_opt_abort(ioc);
return -1; return -1;
} }
g_free(name); g_free(name);
@ -690,6 +698,12 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc,
if (reply.type != NBD_REP_ACK) { if (reply.type != NBD_REP_ACK) {
error_setg(errp, "Unexpected reply type %" PRIx32 " expected %x", error_setg(errp, "Unexpected reply type %" PRIx32 " expected %x",
reply.type, NBD_REP_ACK); reply.type, NBD_REP_ACK);
nbd_send_opt_abort(ioc);
return -1;
}
if (reply.length) {
error_setg(errp, "Unexpected length to ACK response");
nbd_send_opt_abort(ioc);
return -1; return -1;
} }