From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, qemu-devel@nongnu.org
Subject: [Qemu-devel] [PULL 17/19] nbd: Don't fail handshake on NBD_OPT_LIST descriptions
Date: Fri, 15 Apr 2016 19:02:20 +0200 [thread overview]
Message-ID: <1460739742-5315-18-git-send-email-kwolf@redhat.com> (raw)
In-Reply-To: <1460739742-5315-1-git-send-email-kwolf@redhat.com>
From: Eric Blake <eblake@redhat.com>
The NBD Protocol states that NBD_REP_SERVER may set
'length > sizeof(namelen) + namelen'; in which case the rest
of the packet is a UTF-8 description of the export. While we
don't know of any NBD servers that send this description yet,
we had better consume the data so we don't choke when we start
to talk to such a server.
Also, a (buggy/malicious) server that replies with length <
sizeof(namelen) would cause us to block waiting for bytes that
the server is not sending, and one that replies with super-huge
lengths could cause us to temporarily allocate up to 4G memory.
Sanity check things before blindly reading incorrectly.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-id: 1460077777-31004-1-git-send-email-eblake@redhat.com
Reviewed-by: Alex Bligh <alex@alex.org.uk>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
nbd/client.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/nbd/client.c b/nbd/client.c
index 6777e58..48f2a21 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -192,13 +192,18 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
return -1;
}
} else if (type == NBD_REP_SERVER) {
+ if (len < sizeof(namelen) || len > NBD_MAX_BUFFER_SIZE) {
+ error_setg(errp, "incorrect option length");
+ return -1;
+ }
if (read_sync(ioc, &namelen, sizeof(namelen)) != sizeof(namelen)) {
error_setg(errp, "failed to read option name length");
return -1;
}
namelen = be32_to_cpu(namelen);
- if (len != (namelen + sizeof(namelen))) {
- error_setg(errp, "incorrect option mame length");
+ len -= sizeof(namelen);
+ if (len < namelen) {
+ error_setg(errp, "incorrect option name length");
return -1;
}
if (namelen > 255) {
@@ -214,6 +219,20 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, Error **errp)
return -1;
}
(*name)[namelen] = '\0';
+ len -= namelen;
+ if (len) {
+ char *buf = g_malloc(len + 1);
+ if (read_sync(ioc, buf, len) != len) {
+ error_setg(errp, "failed to read export description");
+ g_free(*name);
+ g_free(buf);
+ *name = NULL;
+ return -1;
+ }
+ buf[len] = '\0';
+ TRACE("Ignoring export description: %s", buf);
+ g_free(buf);
+ }
} else {
error_setg(errp, "Unexpected reply type %x expected %x",
type, NBD_REP_SERVER);
--
1.8.3.1
next prev parent reply other threads:[~2016-04-15 17:03 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-15 17:02 [Qemu-devel] [PULL 00/19] Block layer patches for 2.6.0-rc3 Kevin Wolf
2016-04-15 17:02 ` [Qemu-devel] [PULL 01/19] qemu-io: Support 'aio_write -z' Kevin Wolf
2016-04-15 17:02 ` [Qemu-devel] [PULL 02/19] block: Fix blk_aio_write_zeroes() Kevin Wolf
2016-04-15 17:02 ` [Qemu-devel] [PULL 03/19] block/vpc: set errp in vpc_create Kevin Wolf
2016-04-15 17:02 ` [Qemu-devel] [PULL 04/19] vpc: use current_size field for XenServer VHD images Kevin Wolf
2016-04-15 17:02 ` [Qemu-devel] [PULL 05/19] block/vpc: use current_size field for XenConverter " Kevin Wolf
2016-04-15 17:02 ` [Qemu-devel] [PULL 06/19] block/vpc: Use the correct max sector count for " Kevin Wolf
2016-04-15 17:02 ` [Qemu-devel] [PULL 07/19] block/vpc: make checks on max table size a bit more lax Kevin Wolf
2016-04-15 17:02 ` [Qemu-devel] [PULL 08/19] block/vpc: set errp in vpc_open Kevin Wolf
2016-04-15 17:02 ` [Qemu-devel] [PULL 09/19] block/vpc: update comments to be compliant w/coding guidelines Kevin Wolf
2016-04-15 17:02 ` [Qemu-devel] [PULL 10/19] block: Don't ignore flags in blk_{, co, aio}_write_zeroes() Kevin Wolf
2016-04-15 17:02 ` [Qemu-devel] [PULL 11/19] Fix pflash migration Kevin Wolf
2016-04-15 17:02 ` [Qemu-devel] [PULL 12/19] qemu-iotests: drop unused _within_tolerance() filter Kevin Wolf
2016-04-15 17:02 ` [Qemu-devel] [PULL 13/19] qemu-iotests: common.rc: drop unused _do() Kevin Wolf
2016-04-15 17:02 ` [Qemu-devel] [PULL 14/19] qemu-iotests: tests: do not set unused tmp variable Kevin Wolf
2016-04-15 17:02 ` [Qemu-devel] [PULL 15/19] qemu-iotests: place valgrind log file in scratch dir Kevin Wolf
2016-04-15 17:02 ` [Qemu-devel] [PULL 16/19] qemu-iotests: 041: More robust assertion on quorum node Kevin Wolf
2016-04-15 17:02 ` Kevin Wolf [this message]
2016-04-15 17:02 ` [Qemu-devel] [PULL 18/19] nbd: fix assert() on qemu-nbd stop Kevin Wolf
2016-04-15 17:02 ` [Qemu-devel] [PULL 19/19] nbd: Don't kill server on client that doesn't request TLS Kevin Wolf
2016-04-18 8:54 ` [Qemu-devel] [PULL 00/19] Block layer patches for 2.6.0-rc3 Peter Maydell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1460739742-5315-18-git-send-email-kwolf@redhat.com \
--to=kwolf@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).