qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH 1/2] nbd: Drop connection if broken server is de


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-block] [PATCH 1/2] nbd: Drop connection if broken server is detected
Date: Fri, 11 Aug 2017 10:48:48 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1

11.08.2017 05:37, Eric Blake wrote:
As soon as the server is sending us garbage, we should quit
trying to send further messages to the server, and allow all
pending coroutines for any remaining replies to error out.
Failure to do so can let a malicious server cause the client
to hang, for example, if the server sends an invalid magic
number in its response.

Reported by: Vladimir Sementsov-Ogievskiy <address@hidden>
Signed-off-by: Eric Blake <address@hidden>
---
  block/nbd-client.c | 9 +++++++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 25dd28406b..802d50b636 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -68,7 +68,8 @@ static void nbd_teardown_connection(BlockDriverState *bs)

  static coroutine_fn void nbd_read_reply_entry(void *opaque)
  {
-    NBDClientSession *s = opaque;
+    BlockDriverState *bs = opaque;
+    NBDClientSession *s = nbd_get_client_session(bs);
      uint64_t i;
      int ret;
      Error *local_err = NULL;
@@ -107,8 +108,12 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
          qemu_coroutine_yield();
      }

+    s->reply.handle = 0;
      nbd_recv_coroutines_enter_all(s);
      s->read_reply_co = NULL;
+    if (ret < 0) {
+        nbd_teardown_connection(bs);
+    }

what if it happens in parallel with nbd_co_send_request? nbd_teardown_connectin destroys s->ioc, nbd_co_send_requests checks s->ioc only once and then calls nbd_send_request (which is finally nbd_rwv and may yield). I think nbd_rwv is not
prepared to sudden destruction of ioc..

  }

  static int nbd_co_send_request(BlockDriverState *bs,
@@ -416,7 +421,7 @@ int nbd_client_init(BlockDriverState *bs,
      /* Now that we're connected, set the socket to be non-blocking and
       * kick the reply mechanism.  */
      qio_channel_set_blocking(QIO_CHANNEL(sioc), false, NULL);
-    client->read_reply_co = qemu_coroutine_create(nbd_read_reply_entry, 
client);
+    client->read_reply_co = qemu_coroutine_create(nbd_read_reply_entry, bs);
      nbd_client_attach_aio_context(bs, bdrv_get_aio_context(bs));

      logout("Established connection with NBD server\n");


--
Best regards,
Vladimir




reply via email to

[Prev in Thread] Current Thread [Next in Thread]