debian/patches/extra/0044-nbd-client-avoid-read_reply_co-entry-if-send-failed.patch

   1 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
   2 From: Eric Blake <eblake@redhat.com>
   3 Date: Wed, 27 Sep 2017 17:57:22 +0200
   4 Subject: [PATCH] nbd-client: avoid read_reply_co entry if send failed
   5
   6 RH-Author: Eric Blake <eblake@redhat.com>
   7 Message-id: <20170927175725.20023-5-eblake@redhat.com>
   8 Patchwork-id: 76674
   9 O-Subject: [RHEV-7.4.z qemu-kvm-rhev PATCH 4/7] nbd-client: avoid read_reply_co entry if send failed
  10 Bugzilla: 1495474
  11 RH-Acked-by: Max Reitz <mreitz@redhat.com>
  12 RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
  13 RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
  14
  15 From: Stefan Hajnoczi <stefanha@redhat.com>
  16
  17 The following segfault is encountered if the NBD server closes the UNIX
  18 domain socket immediately after negotiation:
  19
  20   Program terminated with signal SIGSEGV, Segmentation fault.
  21   #0  aio_co_schedule (ctx=0x0, co=0xd3c0ff2ef0) at util/async.c:441
  22   441       QSLIST_INSERT_HEAD_ATOMIC(&ctx->scheduled_coroutines,
  23   (gdb) bt
  24   #0  0x000000d3c01a50f8 in aio_co_schedule (ctx=0x0, co=0xd3c0ff2ef0) at util/async.c:441
  25   #1  0x000000d3c012fa90 in nbd_coroutine_end (bs=bs@entry=0xd3c0fec650, request=<optimized out>) at block/nbd-client.c:207
  26   #2  0x000000d3c012fb58 in nbd_client_co_preadv (bs=0xd3c0fec650, offset=0, bytes=<optimized out>, qiov=0x7ffc10a91b20, flags=0) at block/nbd-client.c:237
  27   #3  0x000000d3c0128e63 in bdrv_driver_preadv (bs=bs@entry=0xd3c0fec650, offset=offset@entry=0, bytes=bytes@entry=512, qiov=qiov@entry=0x7ffc10a91b20, flags=0) at block/io.c:836
  28   #4  0x000000d3c012c3e0 in bdrv_aligned_preadv (child=child@entry=0xd3c0ff51d0, req=req@entry=0x7f31885d6e90, offset=offset@entry=0, bytes=bytes@entry=512, align=align@entry=1, qiov=qiov@entry=0x7ffc10a91b20, f
  29 +lags=0) at block/io.c:1086
  30   #5  0x000000d3c012c6b8 in bdrv_co_preadv (child=0xd3c0ff51d0, offset=offset@entry=0, bytes=bytes@entry=512, qiov=qiov@entry=0x7ffc10a91b20, flags=flags@entry=0) at block/io.c:1182
  31   #6  0x000000d3c011cc17 in blk_co_preadv (blk=0xd3c0ff4f80, offset=0, bytes=512, qiov=0x7ffc10a91b20, flags=0) at block/block-backend.c:1032
  32   #7  0x000000d3c011ccec in blk_read_entry (opaque=0x7ffc10a91b40) at block/block-backend.c:1079
  33   #8  0x000000d3c01bbb96 in coroutine_trampoline (i0=<optimized out>, i1=<optimized out>) at util/coroutine-ucontext.c:79
  34   #9  0x00007f3196cb8600 in __start_context () at /lib64/libc.so.6
  35
  36 The problem is that nbd_client_init() uses
  37 nbd_client_attach_aio_context() -> aio_co_schedule(new_context,
  38 client->read_reply_co).  Execution of read_reply_co is deferred to a BH
  39 which doesn't run until later.
  40
  41 In the mean time blk_co_preadv() can be called and nbd_coroutine_end()
  42 calls aio_wake() on read_reply_co.  At this point in time
  43 read_reply_co's ctx isn't set because it has never been entered yet.
  44
  45 This patch simplifies the nbd_co_send_request() ->
  46 nbd_co_receive_reply() -> nbd_coroutine_end() lifecycle to just
  47 nbd_co_send_request() -> nbd_co_receive_reply().  The request is "ended"
  48 if an error occurs at any point.  Callers no longer have to invoke
  49 nbd_coroutine_end().
  50
  51 This cleanup also eliminates the segfault because we don't call
  52 aio_co_schedule() to wake up s->read_reply_co if sending the request
  53 failed.  It is only necessary to wake up s->read_reply_co if a reply was
  54 received.
  55
  56 Note this only happens with UNIX domain sockets on Linux.  It doesn't
  57 seem possible to reproduce this with TCP sockets.
  58
  59 Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
  60 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
  61 Message-Id: <20170829122745.14309-2-stefanha@redhat.com>
  62 Signed-off-by: Eric Blake <eblake@redhat.com>
  63 (cherry picked from commit 3c2d5183f9fa4eac3d17d841e26da65a0181ae7b)
  64 Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
  65 ---
  66  block/nbd-client.c | 25 +++++++++----------------
  67  1 file changed, 9 insertions(+), 16 deletions(-)
  68
  69 diff --git a/block/nbd-client.c b/block/nbd-client.c
  70 index f7bca3f996..434acf647f 100644
  71 --- a/block/nbd-client.c
  72 +++ b/block/nbd-client.c
  73 @@ -139,12 +139,12 @@ static int nbd_co_send_request(BlockDriverState *bs,
  74      request->handle = INDEX_TO_HANDLE(s, i);
  75
  76      if (s->quit) {
  77 -        qemu_co_mutex_unlock(&s->send_mutex);
  78 -        return -EIO;
  79 +        rc = -EIO;
  80 +        goto err;
  81      }
  82      if (!s->ioc) {
  83 -        qemu_co_mutex_unlock(&s->send_mutex);
  84 -        return -EPIPE;
  85 +        rc = -EPIPE;
  86 +        goto err;
  87      }
  88
  89      if (qiov) {
  90 @@ -161,8 +161,13 @@ static int nbd_co_send_request(BlockDriverState *bs,
  91      } else {
  92          rc = nbd_send_request(s->ioc, request);
  93      }
  94 +
  95 +err:
  96      if (rc < 0) {
  97          s->quit = true;
  98 +        s->requests[i].coroutine = NULL;
  99 +        s->in_flight--;
 100 +        qemu_co_queue_next(&s->free_sema);
 101      }
 102      qemu_co_mutex_unlock(&s->send_mutex);
 103      return rc;
 104 @@ -196,13 +201,6 @@ static void nbd_co_receive_reply(NBDClientSession *s,
 105          /* Tell the read handler to read another header.  */
 106          s->reply.handle = 0;
 107      }
 108 -}
 109 -
 110 -static void nbd_coroutine_end(BlockDriverState *bs,
 111 -                              NBDRequest *request)
 112 -{
 113 -    NBDClientSession *s = nbd_get_client_session(bs);
 114 -    int i = HANDLE_TO_INDEX(s, request->handle);
 115
 116      s->requests[i].coroutine = NULL;
 117
 118 @@ -238,7 +236,6 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
 119      } else {
 120          nbd_co_receive_reply(client, &request, &reply, qiov);
 121      }
 122 -    nbd_coroutine_end(bs, &request);
 123      return -reply.error;
 124  }
 125
 126 @@ -267,7 +264,6 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
 127      } else {
 128          nbd_co_receive_reply(client, &request, &reply, NULL);
 129      }
 130 -    nbd_coroutine_end(bs, &request);
 131      return -reply.error;
 132  }
 133
 134 @@ -301,7 +297,6 @@ int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
 135      } else {
 136          nbd_co_receive_reply(client, &request, &reply, NULL);
 137      }
 138 -    nbd_coroutine_end(bs, &request);
 139      return -reply.error;
 140  }
 141
 142 @@ -325,7 +320,6 @@ int nbd_client_co_flush(BlockDriverState *bs)
 143      } else {
 144          nbd_co_receive_reply(client, &request, &reply, NULL);
 145      }
 146 -    nbd_coroutine_end(bs, &request);
 147      return -reply.error;
 148  }
 149
 150 @@ -350,7 +344,6 @@ int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int count)
 151      } else {
 152          nbd_co_receive_reply(client, &request, &reply, NULL);
 153      }
 154 -    nbd_coroutine_end(bs, &request);
 155      return -reply.error;
 156
 157  }
 158 --
 159 2.11.0
 160