From b202cbe30ea881c40defe4e7dde664e0b99e8c06 Mon Sep 17 00:00:00 2001

From: Eric Blake <eblake@redhat.com>

Date: Wed, 19 Jul 2017 18:02:02 +0200

Subject: [PATCH 05/17] nbd/server: get rid of nbd_negotiate_read and friends

RH-Author: Eric Blake <eblake@redhat.com>

Message-id: <20170719180202.23329-5-eblake@redhat.com>

Patchwork-id: 75816

O-Subject: [RHEV-7.4.z qemu-kvm-rhev PATCH 4/4] nbd/server: get rid of nbd_negotiate_read and friends

Bugzilla: 1473638

RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>

RH-Acked-by: Jeffrey Cody <jcody@redhat.com>

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Functions nbd_negotiate_{read,write,drop_sync} were introduced in

1a6245a5b, when nbd_rwv (was nbd_wr_sync) was working through

qemu_co_sendv_recvv (the path is nbd_wr_sync -> qemu_co_{recv/send} ->

qemu_co_send_recv -> qemu_co_sendv_recvv), which just yields, without

setting any handlers. But starting from ff82911cd nbd_rwv (was

nbd_wr_syncv) works through qio_channel_yield() which sets handlers, so

watchers are redundant in nbd_negotiate_{read,write,drop_sync}, then,

let's just use nbd_{read,write,drop} functions.

Functions nbd_{read,write,drop} has errp parameter, which is unused in

this patch. This will be fixed later.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Reviewed-by: Eric Blake <eblake@redhat.com>

Message-Id: <20170602150150.258222-4-vsementsov@virtuozzo.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

(cherry picked from commit 2b0bbc4f8809c972bad134bc1a2570dbb01dea0b)

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

Conflicts:

	nbd/server.c - missing errp addition (e44ed99) and bulk

	rename (d1fdf25)

Fixes CVE-2017-7539

Signed-off-by: Eric Blake <eblake@redhat.com>

---

 nbd/server.c | 106 ++++++++++++-----------------------------------------------

 1 file changed, 21 insertions(+), 85 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c

index b44cbe6..8e3b8e5 100644

--- a/nbd/server.c

+++ b/nbd/server.c

@@ -104,69 +104,6 @@ struct NBDClient {

 static void nbd_client_receive_next_request(NBDClient *client);

-static gboolean nbd_negotiate_continue(QIOChannel *ioc,

-                                       GIOCondition condition,

-                                       void *opaque)

-{

-    qemu_coroutine_enter(opaque);

-    return TRUE;

-}

-

-static int nbd_negotiate_read(QIOChannel *ioc, void *buffer, size_t size)

-{

-    ssize_t ret;

-    guint watch;

-

-    assert(qemu_in_coroutine());

-    /* Negotiation are always in main loop. */

-    watch = qio_channel_add_watch(ioc,

-                                  G_IO_IN,

-                                  nbd_negotiate_continue,

-                                  qemu_coroutine_self(),

-                                  NULL);

-    ret = read_sync(ioc, buffer, size);

-    g_source_remove(watch);

-    return ret;

-

-}

-

-static int nbd_negotiate_write(QIOChannel *ioc, const void *buffer, size_t size)

-{

-    ssize_t ret;

-    guint watch;

-

-    assert(qemu_in_coroutine());

-    /* Negotiation are always in main loop. */

-    watch = qio_channel_add_watch(ioc,

-                                  G_IO_OUT,

-                                  nbd_negotiate_continue,

-                                  qemu_coroutine_self(),

-                                  NULL);

-    ret = write_sync(ioc, buffer, size);

-    g_source_remove(watch);

-    return ret;

-}

-

-static int nbd_negotiate_drop_sync(QIOChannel *ioc, size_t size)

-{

-    ssize_t ret;

-    uint8_t *buffer = g_malloc(MIN(65536, size));

-

-    while (size > 0) {

-        size_t count = MIN(65536, size);

-        ret = nbd_negotiate_read(ioc, buffer, count);

-        if (ret < 0) {

-            g_free(buffer);

-            return ret;

-        }

-

-        size -= count;

-    }

-

-    g_free(buffer);

-    return 0;

-}

-

 /* Basic flow for negotiation

    Server         Client

@@ -205,22 +142,22 @@ static int nbd_negotiate_send_rep_len(QIOChannel *ioc, uint32_t type,

           type, opt, len);

     magic = cpu_to_be64(NBD_REP_MAGIC);

-    if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) < 0) {

+    if (write_sync(ioc, &magic, sizeof(magic)) < 0) {

         LOG("write failed (rep magic)");

         return -EINVAL;

     }

     opt = cpu_to_be32(opt);

-    if (nbd_negotiate_write(ioc, &opt, sizeof(opt)) < 0) {

+    if (write_sync(ioc, &opt, sizeof(opt)) < 0) {

         LOG("write failed (rep opt)");

         return -EINVAL;

     }

     type = cpu_to_be32(type);

-    if (nbd_negotiate_write(ioc, &type, sizeof(type)) < 0) {

+    if (write_sync(ioc, &type, sizeof(type)) < 0) {

         LOG("write failed (rep type)");

         return -EINVAL;

     }

     len = cpu_to_be32(len);

-    if (nbd_negotiate_write(ioc, &len, sizeof(len)) < 0) {

+    if (write_sync(ioc, &len, sizeof(len)) < 0) {

         LOG("write failed (rep data length)");

         return -EINVAL;

     }

@@ -255,7 +192,7 @@ nbd_negotiate_send_rep_err(QIOChannel *ioc, uint32_t type,

     if (ret < 0) {

         goto out;

     }

-    if (nbd_negotiate_write(ioc, msg, len) < 0) {

+    if (write_sync(ioc, msg, len) < 0) {

         LOG("write failed (error message)");

         ret = -EIO;

     } else {

@@ -286,15 +223,15 @@ static int nbd_negotiate_send_rep_list(QIOChannel *ioc, NBDExport *exp)

     }

     len = cpu_to_be32(name_len);

-    if (nbd_negotiate_write(ioc, &len, sizeof(len)) < 0) {

+    if (write_sync(ioc, &len, sizeof(len)) < 0) {

         LOG("write failed (name length)");

         return -EINVAL;

     }

-    if (nbd_negotiate_write(ioc, name, name_len) < 0) {

+    if (write_sync(ioc, name, name_len) < 0) {

         LOG("write failed (name buffer)");

         return -EINVAL;

     }

-    if (nbd_negotiate_write(ioc, desc, desc_len) < 0) {

+    if (write_sync(ioc, desc, desc_len) < 0) {

         LOG("write failed (description buffer)");

         return -EINVAL;

     }

@@ -308,7 +245,7 @@ static int nbd_negotiate_handle_list(NBDClient *client, uint32_t length)

     NBDExport *exp;

     if (length) {

-        if (nbd_negotiate_drop_sync(client->ioc, length) < 0) {

+        if (nbd_drop(client->ioc, length) < 0) {

             return -EIO;

         }

         return nbd_negotiate_send_rep_err(client->ioc,

@@ -339,7 +276,7 @@ static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t length)

         LOG("Bad length received");

         goto fail;

     }

-    if (nbd_negotiate_read(client->ioc, name, length) < 0) {

+    if (read_sync(client->ioc, name, length) < 0) {

         LOG("read failed");

         goto fail;

     }

@@ -372,7 +309,7 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client,

     TRACE("Setting up TLS");

     ioc = client->ioc;

     if (length) {

-        if (nbd_negotiate_drop_sync(ioc, length) < 0) {

+        if (nbd_drop(ioc, length) < 0) {

             return NULL;

         }

         nbd_negotiate_send_rep_err(ioc, NBD_REP_ERR_INVALID, NBD_OPT_STARTTLS,

@@ -436,7 +373,7 @@ static int nbd_negotiate_options(NBDClient *client)

         ...           Rest of request

     */

-    if (nbd_negotiate_read(client->ioc, &flags, sizeof(flags)) < 0) {

+    if (read_sync(client->ioc, &flags, sizeof(flags)) < 0) {

         LOG("read failed");

         return -EIO;

     }

@@ -462,7 +399,7 @@ static int nbd_negotiate_options(NBDClient *client)

         uint32_t clientflags, length;

         uint64_t magic;

-        if (nbd_negotiate_read(client->ioc, &magic, sizeof(magic)) < 0) {

+        if (read_sync(client->ioc, &magic, sizeof(magic)) < 0) {

             LOG("read failed");

             return -EINVAL;

         }

@@ -472,15 +409,14 @@ static int nbd_negotiate_options(NBDClient *client)

             return -EINVAL;

         }

-        if (nbd_negotiate_read(client->ioc, &clientflags,

-                               sizeof(clientflags)) < 0)

+        if (read_sync(client->ioc, &clientflags, sizeof(clientflags)) < 0)

         {

             LOG("read failed");

             return -EINVAL;

         }

         clientflags = be32_to_cpu(clientflags);

-        if (nbd_negotiate_read(client->ioc, &length, sizeof(length)) < 0) {

+        if (read_sync(client->ioc, &length, sizeof(length)) < 0) {

             LOG("read failed");

             return -EINVAL;

         }

@@ -510,7 +446,7 @@ static int nbd_negotiate_options(NBDClient *client)

                 return -EINVAL;

             default:

-                if (nbd_negotiate_drop_sync(client->ioc, length) < 0) {

+                if (nbd_drop(client->ioc, length) < 0) {

                     return -EIO;

                 }

                 ret = nbd_negotiate_send_rep_err(client->ioc,

@@ -548,7 +484,7 @@ static int nbd_negotiate_options(NBDClient *client)

                 return nbd_negotiate_handle_export_name(client, length);

             case NBD_OPT_STARTTLS:

-                if (nbd_negotiate_drop_sync(client->ioc, length) < 0) {

+                if (nbd_drop(client->ioc, length) < 0) {

                     return -EIO;

                 }

                 if (client->tlscreds) {

@@ -567,7 +503,7 @@ static int nbd_negotiate_options(NBDClient *client)

                 }

                 break;

             default:

-                if (nbd_negotiate_drop_sync(client->ioc, length) < 0) {

+                if (nbd_drop(client->ioc, length) < 0) {

                     return -EIO;

                 }

                 ret = nbd_negotiate_send_rep_err(client->ioc,

@@ -656,12 +592,12 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)

             TRACE("TLS cannot be enabled with oldstyle protocol");

             goto fail;

         }

-        if (nbd_negotiate_write(client->ioc, buf, sizeof(buf)) < 0) {

+        if (write_sync(client->ioc, buf, sizeof(buf)) < 0) {

             LOG("write failed");

             goto fail;

         }

     } else {

-        if (nbd_negotiate_write(client->ioc, buf, 18) < 0) {

+        if (write_sync(client->ioc, buf, 18) < 0) {

             LOG("write failed");

             goto fail;

         }

@@ -676,7 +612,7 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData *data)

         stq_be_p(buf + 18, client->exp->size);

         stw_be_p(buf + 26, client->exp->nbdflags | myflags);

         len = client->no_zeroes ? 10 : sizeof(buf) - 18;

-        if (nbd_negotiate_write(client->ioc, buf + 18, len) < 0) {

+        if (write_sync(client->ioc, buf + 18, len) < 0) {

             LOG("write failed");

             goto fail;

         }

-- 

1.8.3.1