[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH for-2.6] nbd: Don't fail handshake
From: |
Alex Bligh |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH for-2.6] nbd: Don't fail handshake on NBD_OPT_LIST descriptions |
Date: |
Thu, 14 Apr 2016 16:46:44 +0100 |
On 14 Apr 2016, at 16:26, Eric Blake <address@hidden> wrote:
> [adding qemu-block in cc, since Paolo can't send pull request]
>
> On 04/07/2016 07:09 PM, Eric Blake wrote:
>> 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 <address@hidden>
Reviewed-by: Alex Bligh <address@hidden>
>> ---
>>
>> Yet another case of code introduced in 2.6 that doesn't play
>> nicely with spec-compliant servers...
>>
>> Hopefully I've squashed them all now?
>>
>> 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);
>>
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
--
Alex Bligh
signature.asc
Description: Message signed with OpenPGP using GPGMail