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
6 RH-Author: Eric Blake <eblake@redhat.com>
7 Message-id: <20170927175725.20023-5-eblake@redhat.com>
9 O-Subject: [RHEV-7.4.z qemu-kvm-rhev PATCH 4/7] nbd-client: avoid read_reply_co entry if send failed
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>
15 From: Stefan Hajnoczi <stefanha@redhat.com>
17 The following segfault is encountered if the NBD server closes the UNIX
18 domain socket immediately after negotiation:
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,
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
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.
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.
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
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
56 Note this only happens with UNIX domain sockets on Linux. It doesn't
57 seem possible to reproduce this with TCP sockets.
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>
66 block/nbd-client.c | 25 +++++++++----------------
67 1 file changed, 9 insertions(+), 16 deletions(-)
69 diff --git a/block/nbd-client.c b/block/nbd-client.c
70 index f7bca3f..434acf6 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);
77 - qemu_co_mutex_unlock(&s->send_mutex);
83 - qemu_co_mutex_unlock(&s->send_mutex);
90 @@ -161,8 +161,13 @@ static int nbd_co_send_request(BlockDriverState *bs,
92 rc = nbd_send_request(s->ioc, request);
98 + s->requests[i].coroutine = NULL;
100 + qemu_co_queue_next(&s->free_sema);
102 qemu_co_mutex_unlock(&s->send_mutex);
104 @@ -196,13 +201,6 @@ static void nbd_co_receive_reply(NBDClientSession *s,
105 /* Tell the read handler to read another header. */
110 -static void nbd_coroutine_end(BlockDriverState *bs,
111 - NBDRequest *request)
113 - NBDClientSession *s = nbd_get_client_session(bs);
114 - int i = HANDLE_TO_INDEX(s, request->handle);
116 s->requests[i].coroutine = NULL;
118 @@ -238,7 +236,6 @@ int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
120 nbd_co_receive_reply(client, &request, &reply, qiov);
122 - nbd_coroutine_end(bs, &request);
126 @@ -267,7 +264,6 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_t offset,
128 nbd_co_receive_reply(client, &request, &reply, NULL);
130 - nbd_coroutine_end(bs, &request);
134 @@ -301,7 +297,6 @@ int nbd_client_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
136 nbd_co_receive_reply(client, &request, &reply, NULL);
138 - nbd_coroutine_end(bs, &request);
142 @@ -325,7 +320,6 @@ int nbd_client_co_flush(BlockDriverState *bs)
144 nbd_co_receive_reply(client, &request, &reply, NULL);
146 - nbd_coroutine_end(bs, &request);
150 @@ -350,7 +344,6 @@ int nbd_client_co_pdiscard(BlockDriverState *bs, int64_t offset, int count)
152 nbd_co_receive_reply(client, &request, &reply, NULL);
154 - nbd_coroutine_end(bs, &request);