]> git.proxmox.com Git - pve-qemu.git/blame - debian/patches/extra/0041-nbd-client-Fix-regression-when-server-sends-garbage.patch
bump version to 2.9.1-9
[pve-qemu.git] / debian / patches / extra / 0041-nbd-client-Fix-regression-when-server-sends-garbage.patch
CommitLineData
b45e13fe
AD
1From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
2From: Eric Blake <eblake@redhat.com>
3Date: Wed, 27 Sep 2017 17:57:19 +0200
4Subject: [PATCH] nbd-client: Fix regression when server sends garbage
5
6RH-Author: Eric Blake <eblake@redhat.com>
7Message-id: <20170927175725.20023-2-eblake@redhat.com>
8Patchwork-id: 76672
9O-Subject: [RHEV-7.4.z qemu-kvm-rhev PATCH 1/7] nbd-client: Fix regression when server sends garbage
10Bugzilla: 1495474
11RH-Acked-by: Max Reitz <mreitz@redhat.com>
12RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
13RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
14
15When we switched NBD to use coroutines for qemu 2.9 (in particular,
16commit a12a712a), we introduced a regression: if a server sends us
17garbage (such as a corrupted magic number), we quit the read loop
18but do not stop sending further queued commands, resulting in the
19client hanging when it never reads the response to those additional
20commands. In qemu 2.8, we properly detected that the server is no
21longer reliable, and cancelled all existing pending commands with
22EIO, then tore down the socket so that all further command attempts
23get EPIPE.
24
25Restore the proper behavior of quitting (almost) all communication
26with a broken server: Once we know we are out of sync or otherwise
27can't trust the server, we must assume that any further incoming
28data is unreliable and therefore end all pending commands with EIO,
29and quit trying to send any further commands. As an exception, we
30still (try to) send NBD_CMD_DISC to let the server know we are going
31away (in part, because it is easier to do that than to further
32refactor nbd_teardown_connection, and in part because it is the
33only command where we do not have to wait for a reply).
34
35Based on a patch by Vladimir Sementsov-Ogievskiy.
36
37A malicious server can be created with the following hack,
38followed by setting NBD_SERVER_DEBUG to a non-zero value in the
39environment when running qemu-nbd:
40
41| --- a/nbd/server.c
42| +++ b/nbd/server.c
43| @@ -919,6 +919,17 @@ static int nbd_send_reply(QIOChannel *ioc, NBDReply *reply, Error **errp)
44| stl_be_p(buf + 4, reply->error);
45| stq_be_p(buf + 8, reply->handle);
46|
47| + static int debug;
48| + static int count;
49| + if (!count++) {
50| + const char *str = getenv("NBD_SERVER_DEBUG");
51| + if (str) {
52| + debug = atoi(str);
53| + }
54| + }
55| + if (debug && !(count % debug)) {
56| + buf[0] = 0;
57| + }
58| return nbd_write(ioc, buf, sizeof(buf), errp);
59| }
60
61Reported-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
62Signed-off-by: Eric Blake <eblake@redhat.com>
63Message-Id: <20170814213426.24681-1-eblake@redhat.com>
64Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
65(cherry picked from commit 72b6ffc76653214b69a94a7b1643ff80df134486)
66Signed-off-by: Eric Blake <eblake@redhat.com>
67Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
68
69Conflicts:
70 block/nbd-client.c
71---
72 block/nbd-client.c | 17 +++++++++++++----
73 block/nbd-client.h | 1 +
74 2 files changed, 14 insertions(+), 4 deletions(-)
75
76diff --git a/block/nbd-client.c b/block/nbd-client.c
507c6de3 77index 282679b4f8..701b4ce2eb 100644
b45e13fe
AD
78--- a/block/nbd-client.c
79+++ b/block/nbd-client.c
80@@ -71,7 +71,7 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
81 uint64_t i;
82 int ret;
83
84- for (;;) {
85+ while (!s->quit) {
86 assert(s->reply.handle == 0);
87 ret = nbd_receive_reply(s->ioc, &s->reply);
88 if (ret <= 0) {
89@@ -102,6 +102,9 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
90 qemu_coroutine_yield();
91 }
92
93+ if (ret < 0) {
94+ s->quit = true;
95+ }
96 nbd_recv_coroutines_enter_all(s);
97 s->read_reply_co = NULL;
98 }
99@@ -130,6 +133,10 @@ static int nbd_co_send_request(BlockDriverState *bs,
100 assert(i < MAX_NBD_REQUESTS);
101 request->handle = INDEX_TO_HANDLE(s, i);
102
103+ if (s->quit) {
104+ qemu_co_mutex_unlock(&s->send_mutex);
105+ return -EIO;
106+ }
107 if (!s->ioc) {
108 qemu_co_mutex_unlock(&s->send_mutex);
109 return -EPIPE;
110@@ -138,7 +145,7 @@ static int nbd_co_send_request(BlockDriverState *bs,
111 if (qiov) {
112 qio_channel_set_cork(s->ioc, true);
113 rc = nbd_send_request(s->ioc, request);
114- if (rc >= 0) {
115+ if (rc >= 0 && !s->quit) {
116 ret = nbd_wr_syncv(s->ioc, qiov->iov, qiov->niov, request->len,
117 false);
118 if (ret != request->len) {
119@@ -149,6 +156,9 @@ static int nbd_co_send_request(BlockDriverState *bs,
120 } else {
121 rc = nbd_send_request(s->ioc, request);
122 }
123+ if (rc < 0) {
124+ s->quit = true;
125+ }
126 qemu_co_mutex_unlock(&s->send_mutex);
127 return rc;
128 }
129@@ -163,8 +173,7 @@ static void nbd_co_receive_reply(NBDClientSession *s,
130 /* Wait until we're woken up by nbd_read_reply_entry. */
131 qemu_coroutine_yield();
132 *reply = s->reply;
133- if (reply->handle != request->handle ||
134- !s->ioc) {
135+ if (reply->handle != request->handle || !s->ioc || s->quit) {
136 reply->error = EIO;
137 } else {
138 if (qiov && reply->error == 0) {
139diff --git a/block/nbd-client.h b/block/nbd-client.h
507c6de3 140index 891ba44a20..9774a8ebbb 100644
b45e13fe
AD
141--- a/block/nbd-client.h
142+++ b/block/nbd-client.h
143@@ -30,6 +30,7 @@ typedef struct NBDClientSession {
144
145 Coroutine *recv_coroutine[MAX_NBD_REQUESTS];
146 NBDReply reply;
147+ bool quit;
148 } NBDClientSession;
149
150 NBDClientSession *nbd_get_client_session(BlockDriverState *bs);
151--
507c6de3 1522.11.0
b45e13fe 153