summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorHitendra Prajapati <hprajapati@mvista.com>2024-09-03 13:38:34 +0530
committerSteve Sakoman <steve@sakoman.com>2024-09-07 05:38:17 -0700
commita952dec3e479f50a645faaf7258f5322c1767e42 (patch)
tree6f88d48d55d8a2121ab23f22cd1cce38ab93f711
parent0ae3b2bd492d32ba401a447f09679ebaf10ddad4 (diff)
downloadpoky-a952dec3e479f50a645faaf7258f5322c1767e42.tar.gz
qemu: fix CVE-2024-7409
A flaw was found in the QEMU NBD Server. This vulnerability allows a denial of service (DoS) attack via improper synchronization during socket closure when a client keeps a socket open as the server is taken offline. Reference: https://nvd.nist.gov/vuln/detail/CVE-2024-7409 Upstream Patches: https://github.com/qemu/qemu/commit/fb1c2aaa981e0a2fa6362c9985f1296b74f055ac https://github.com/qemu/qemu/commit/c8a76dbd90c2f48df89b75bef74917f90a59b623 https://gitlab.com/qemu-project/qemu/-/commit/b9b72cb3ce15b693148bd09cef7e50110566d8a0 https://gitlab.com/qemu-project/qemu/-/commit/3e7ef738c8462c45043a1d39f702a0990406a3b3 (From OE-Core rev: d84ab04dc66cb83638f96fcd2f4c67e67489c410) Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com> Signed-off-by: Steve Sakoman <steve@sakoman.com>
-rw-r--r--meta/recipes-devtools/qemu/qemu.inc4
-rw-r--r--meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0001.patch162
-rw-r--r--meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0002.patch174
-rw-r--r--meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0003.patch122
-rw-r--r--meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0004.patch163
5 files changed, 625 insertions, 0 deletions
diff --git a/meta/recipes-devtools/qemu/qemu.inc b/meta/recipes-devtools/qemu/qemu.inc
index 4747310ae4..4684e44524 100644
--- a/meta/recipes-devtools/qemu/qemu.inc
+++ b/meta/recipes-devtools/qemu/qemu.inc
@@ -109,6 +109,10 @@ SRC_URI = "https://download.qemu.org/${BPN}-${PV}.tar.xz \
109 file://scsi-disk-ensure-block-size-is-non-zero-and-changes-limited-to-bits-8-15.patch \ 109 file://scsi-disk-ensure-block-size-is-non-zero-and-changes-limited-to-bits-8-15.patch \
110 file://CVE-2023-42467.patch \ 110 file://CVE-2023-42467.patch \
111 file://CVE-2023-6683.patch \ 111 file://CVE-2023-6683.patch \
112 file://CVE-2024-7409-0001.patch \
113 file://CVE-2024-7409-0002.patch \
114 file://CVE-2024-7409-0003.patch \
115 file://CVE-2024-7409-0004.patch \
112 " 116 "
113UPSTREAM_CHECK_REGEX = "qemu-(?P<pver>\d+(\.\d+)+)\.tar" 117UPSTREAM_CHECK_REGEX = "qemu-(?P<pver>\d+(\.\d+)+)\.tar"
114 118
diff --git a/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0001.patch b/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0001.patch
new file mode 100644
index 0000000000..f4dad65097
--- /dev/null
+++ b/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0001.patch
@@ -0,0 +1,162 @@
1From fb1c2aaa981e0a2fa6362c9985f1296b74f055ac Mon Sep 17 00:00:00 2001
2From: Eric Blake <eblake@redhat.com>
3Date: Wed, 7 Aug 2024 08:50:01 -0500
4Subject: [PATCH] nbd/server: Plumb in new args to nbd_client_add()
5
6Upcoming patches to fix a CVE need to track an opaque pointer passed
7in by the owner of a client object, as well as request for a time
8limit on how fast negotiation must complete. Prepare for that by
9changing the signature of nbd_client_new() and adding an accessor to
10get at the opaque pointer, although for now the two servers
11(qemu-nbd.c and blockdev-nbd.c) do not change behavior even though
12they pass in a new default timeout value.
13
14Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
15Signed-off-by: Eric Blake <eblake@redhat.com>
16Message-ID: <20240807174943.771624-11-eblake@redhat.com>
17Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
18[eblake: s/LIMIT/MAX_SECS/ as suggested by Dan]
19Signed-off-by: Eric Blake <eblake@redhat.com>
20
21CVE: CVE-2024-7409
22Upstream-Status: Backport [https://github.com/qemu/qemu/commit/fb1c2aaa981e0a2fa6362c9985f1296b74f055ac]
23Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
24---
25 blockdev-nbd.c | 6 ++++--
26 include/block/nbd.h | 11 ++++++++++-
27 nbd/server.c | 20 +++++++++++++++++---
28 qemu-nbd.c | 4 +++-
29 4 files changed, 34 insertions(+), 7 deletions(-)
30
31diff --git a/blockdev-nbd.c b/blockdev-nbd.c
32index bdfa7ed3a..b9e8dc78f 100644
33--- a/blockdev-nbd.c
34+++ b/blockdev-nbd.c
35@@ -59,8 +59,10 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
36 nbd_update_server_watch(nbd_server);
37
38 qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server");
39- nbd_client_new(cioc, nbd_server->tlscreds, nbd_server->tlsauthz,
40- nbd_blockdev_client_closed);
41+ /* TODO - expose handshake timeout as QMP option */
42+ nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_MAX_SECS,
43+ nbd_server->tlscreds, nbd_server->tlsauthz,
44+ nbd_blockdev_client_closed, NULL);
45 }
46
47 static void nbd_update_server_watch(NBDServerData *s)
48diff --git a/include/block/nbd.h b/include/block/nbd.h
49index 78d101b77..b71a29724 100644
50--- a/include/block/nbd.h
51+++ b/include/block/nbd.h
52@@ -27,6 +27,12 @@
53
54 extern const BlockExportDriver blk_exp_nbd;
55
56+/*
57+ * NBD_DEFAULT_HANDSHAKE_MAX_SECS: Number of seconds in which client must
58+ * succeed at NBD_OPT_GO before being forcefully dropped as too slow.
59+ */
60+#define NBD_DEFAULT_HANDSHAKE_MAX_SECS 10
61+
62 /* Handshake phase structs - this struct is passed on the wire */
63
64 struct NBDOption {
65@@ -338,9 +344,12 @@ AioContext *nbd_export_aio_context(NBDExport *exp);
66 NBDExport *nbd_export_find(const char *name);
67
68 void nbd_client_new(QIOChannelSocket *sioc,
69+ uint32_t handshake_max_secs,
70 QCryptoTLSCreds *tlscreds,
71 const char *tlsauthz,
72- void (*close_fn)(NBDClient *, bool));
73+ void (*close_fn)(NBDClient *, bool),
74+ void *owner);
75+void *nbd_client_owner(NBDClient *client);
76 void nbd_client_get(NBDClient *client);
77 void nbd_client_put(NBDClient *client);
78
79diff --git a/nbd/server.c b/nbd/server.c
80index 4630dd732..12680c8dc 100644
81--- a/nbd/server.c
82+++ b/nbd/server.c
83@@ -121,9 +121,11 @@ struct NBDClient {
84 int refcount;
85 void (*close_fn)(NBDClient *client, bool negotiated);
86
87+ void *owner;
88 NBDExport *exp;
89 QCryptoTLSCreds *tlscreds;
90 char *tlsauthz;
91+ uint32_t handshake_max_secs;
92 QIOChannelSocket *sioc; /* The underlying data channel */
93 QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */
94
95@@ -2703,6 +2705,7 @@ static coroutine_fn void nbd_co_client_start(void *opaque)
96
97 qemu_co_mutex_init(&client->send_lock);
98
99+ /* TODO - utilize client->handshake_max_secs */
100 if (nbd_negotiate(client, &local_err)) {
101 if (local_err) {
102 error_report_err(local_err);
103@@ -2715,14 +2718,17 @@ static coroutine_fn void nbd_co_client_start(void *opaque)
104 }
105
106 /*
107- * Create a new client listener using the given channel @sioc.
108+ * Create a new client listener using the given channel @sioc and @owner.
109 * Begin servicing it in a coroutine. When the connection closes, call
110- * @close_fn with an indication of whether the client completed negotiation.
111+ * @close_fn with an indication of whether the client completed negotiation
112+ * within @handshake_max_secs seconds (0 for unbounded).
113 */
114 void nbd_client_new(QIOChannelSocket *sioc,
115+ uint32_t handshake_max_secs,
116 QCryptoTLSCreds *tlscreds,
117 const char *tlsauthz,
118- void (*close_fn)(NBDClient *, bool))
119+ void (*close_fn)(NBDClient *, bool),
120+ void *owner)
121 {
122 NBDClient *client;
123 Coroutine *co;
124@@ -2734,12 +2740,20 @@ void nbd_client_new(QIOChannelSocket *sioc,
125 object_ref(OBJECT(client->tlscreds));
126 }
127 client->tlsauthz = g_strdup(tlsauthz);
128+ client->handshake_max_secs = handshake_max_secs;
129 client->sioc = sioc;
130 object_ref(OBJECT(client->sioc));
131 client->ioc = QIO_CHANNEL(sioc);
132 object_ref(OBJECT(client->ioc));
133 client->close_fn = close_fn;
134+ client->owner = owner;
135
136 co = qemu_coroutine_create(nbd_co_client_start, client);
137 qemu_coroutine_enter(co);
138 }
139+
140+void *
141+nbd_client_owner(NBDClient *client)
142+{
143+ return client->owner;
144+}
145diff --git a/qemu-nbd.c b/qemu-nbd.c
146index c6c20df68..f48abf379 100644
147--- a/qemu-nbd.c
148+++ b/qemu-nbd.c
149@@ -363,7 +363,9 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
150
151 nb_fds++;
152 nbd_update_server_watch();
153- nbd_client_new(cioc, tlscreds, tlsauthz, nbd_client_closed);
154+ /* TODO - expose handshake timeout as command line option */
155+ nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_MAX_SECS,
156+ tlscreds, tlsauthz, nbd_client_closed, NULL);
157 }
158
159 static void nbd_update_server_watch(void)
160--
1612.25.1
162
diff --git a/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0002.patch b/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0002.patch
new file mode 100644
index 0000000000..ccef8b36c5
--- /dev/null
+++ b/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0002.patch
@@ -0,0 +1,174 @@
1From c8a76dbd90c2f48df89b75bef74917f90a59b623 Mon Sep 17 00:00:00 2001
2From: Eric Blake <eblake@redhat.com>
3Date: Tue, 6 Aug 2024 13:53:00 -0500
4Subject: [PATCH] nbd/server: CVE-2024-7409: Cap default max-connections to 100
5
6Allowing an unlimited number of clients to any web service is a recipe
7for a rudimentary denial of service attack: the client merely needs to
8open lots of sockets without closing them, until qemu no longer has
9any more fds available to allocate.
10
11For qemu-nbd, we default to allowing only 1 connection unless more are
12explicitly asked for (-e or --shared); this was historically picked as
13a nice default (without an explicit -t, a non-persistent qemu-nbd goes
14away after a client disconnects, without needing any additional
15follow-up commands), and we are not going to change that interface now
16(besides, someday we want to point people towards qemu-storage-daemon
17instead of qemu-nbd).
18
19But for qemu proper, and the newer qemu-storage-daemon, the QMP
20nbd-server-start command has historically had a default of unlimited
21number of connections, in part because unlike qemu-nbd it is
22inherently persistent until nbd-server-stop. Allowing multiple client
23sockets is particularly useful for clients that can take advantage of
24MULTI_CONN (creating parallel sockets to increase throughput),
25although known clients that do so (such as libnbd's nbdcopy) typically
26use only 8 or 16 connections (the benefits of scaling diminish once
27more sockets are competing for kernel attention). Picking a number
28large enough for typical use cases, but not unlimited, makes it
29slightly harder for a malicious client to perform a denial of service
30merely by opening lots of connections withot progressing through the
31handshake.
32
33This change does not eliminate CVE-2024-7409 on its own, but reduces
34the chance for fd exhaustion or unlimited memory usage as an attack
35surface. On the other hand, by itself, it makes it more obvious that
36with a finite limit, we have the problem of an unauthenticated client
37holding 100 fds opened as a way to block out a legitimate client from
38being able to connect; thus, later patches will further add timeouts
39to reject clients that are not making progress.
40
41This is an INTENTIONAL change in behavior, and will break any client
42of nbd-server-start that was not passing an explicit max-connections
43parameter, yet expects more than 100 simultaneous connections. We are
44not aware of any such client (as stated above, most clients aware of
45MULTI_CONN get by just fine on 8 or 16 connections, and probably cope
46with later connections failing by relying on the earlier connections;
47libvirt has not yet been passing max-connections, but generally
48creates NBD servers with the intent for a single client for the sake
49of live storage migration; meanwhile, the KubeSAN project anticipates
50a large cluster sharing multiple clients [up to 8 per node, and up to
51100 nodes in a cluster], but it currently uses qemu-nbd with an
52explicit --shared=0 rather than qemu-storage-daemon with
53nbd-server-start).
54
55We considered using a deprecation period (declare that omitting
56max-parameters is deprecated, and make it mandatory in 3 releases -
57then we don't need to pick an arbitrary default); that has zero risk
58of breaking any apps that accidentally depended on more than 100
59connections, and where such breakage might not be noticed under unit
60testing but only under the larger loads of production usage. But it
61does not close the denial-of-service hole until far into the future,
62and requires all apps to change to add the parameter even if 100 was
63good enough. It also has a drawback that any app (like libvirt) that
64is accidentally relying on an unlimited default should seriously
65consider their own CVE now, at which point they are going to change to
66pass explicit max-connections sooner than waiting for 3 qemu releases.
67Finally, if our changed default breaks an app, that app can always
68pass in an explicit max-parameters with a larger value.
69
70It is also intentional that the HMP interface to nbd-server-start is
71not changed to expose max-connections (any client needing to fine-tune
72things should be using QMP).
73
74Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
75Signed-off-by: Eric Blake <eblake@redhat.com>
76Message-ID: <20240807174943.771624-12-eblake@redhat.com>
77Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
78[ericb: Expand commit message to summarize Dan's argument for why we
79break corner-case back-compat behavior without a deprecation period]
80Signed-off-by: Eric Blake <eblake@redhat.com>
81
82CVE: CVE-2024-7409
83Upstream-Status: Backport [https://github.com/qemu/qemu/commit/c8a76dbd90c2f48df89b75bef74917f90a59b623]
84Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
85---
86 block/monitor/block-hmp-cmds.c | 3 ++-
87 blockdev-nbd.c | 8 ++++++++
88 include/block/nbd.h | 7 +++++++
89 qapi/block-export.json | 4 ++--
90 4 files changed, 19 insertions(+), 3 deletions(-)
91
92diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
93index 2ac4aedff..32a666b5d 100644
94--- a/block/monitor/block-hmp-cmds.c
95+++ b/block/monitor/block-hmp-cmds.c
96@@ -411,7 +411,8 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
97 goto exit;
98 }
99
100- nbd_server_start(addr, NULL, NULL, 0, &local_err);
101+ nbd_server_start(addr, NULL, NULL, NBD_DEFAULT_MAX_CONNECTIONS,
102+ &local_err);
103 qapi_free_SocketAddress(addr);
104 if (local_err != NULL) {
105 goto exit;
106diff --git a/blockdev-nbd.c b/blockdev-nbd.c
107index b9e8dc78f..4bd90bac1 100644
108--- a/blockdev-nbd.c
109+++ b/blockdev-nbd.c
110@@ -171,6 +171,10 @@ void nbd_server_start(SocketAddress *addr, const char *tls_creds,
111
112 void nbd_server_start_options(NbdServerOptions *arg, Error **errp)
113 {
114+ if (!arg->has_max_connections) {
115+ arg->max_connections = NBD_DEFAULT_MAX_CONNECTIONS;
116+ }
117+
118 nbd_server_start(arg->addr, arg->tls_creds, arg->tls_authz,
119 arg->max_connections, errp);
120 }
121@@ -183,6 +187,10 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr,
122 {
123 SocketAddress *addr_flat = socket_address_flatten(addr);
124
125+ if (!has_max_connections) {
126+ max_connections = NBD_DEFAULT_MAX_CONNECTIONS;
127+ }
128+
129 nbd_server_start(addr_flat, tls_creds, tls_authz, max_connections, errp);
130 qapi_free_SocketAddress(addr_flat);
131 }
132diff --git a/include/block/nbd.h b/include/block/nbd.h
133index b71a29724..a31c34a8a 100644
134--- a/include/block/nbd.h
135+++ b/include/block/nbd.h
136@@ -33,6 +33,13 @@ extern const BlockExportDriver blk_exp_nbd;
137 */
138 #define NBD_DEFAULT_HANDSHAKE_MAX_SECS 10
139
140+/*
141+ * NBD_DEFAULT_MAX_CONNECTIONS: Number of client sockets to allow at
142+ * once; must be large enough to allow a MULTI_CONN-aware client like
143+ * nbdcopy to create its typical number of 8-16 sockets.
144+ */
145+#define NBD_DEFAULT_MAX_CONNECTIONS 100
146+
147 /* Handshake phase structs - this struct is passed on the wire */
148
149 struct NBDOption {
150diff --git a/qapi/block-export.json b/qapi/block-export.json
151index c1b92ce1c..181d7238f 100644
152--- a/qapi/block-export.json
153+++ b/qapi/block-export.json
154@@ -21,7 +21,7 @@
155 # recreated on the fly while the NBD server is active.
156 # If missing, it will default to denying access (since 4.0).
157 # @max-connections: The maximum number of connections to allow at the same
158-# time, 0 for unlimited. (since 5.2; default: 0)
159+# time, 0 for unlimited. (since 5.2; default: 100)
160 #
161 # Since: 4.2
162 ##
163@@ -50,7 +50,7 @@
164 # recreated on the fly while the NBD server is active.
165 # If missing, it will default to denying access (since 4.0).
166 # @max-connections: The maximum number of connections to allow at the same
167-# time, 0 for unlimited. (since 5.2; default: 0)
168+# time, 0 for unlimited. (since 5.2; default: 100)
169 #
170 # Returns: error if the server is already running.
171 #
172--
1732.25.1
174
diff --git a/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0003.patch b/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0003.patch
new file mode 100644
index 0000000000..1d27f4712c
--- /dev/null
+++ b/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0003.patch
@@ -0,0 +1,122 @@
1From b9b72cb3ce15b693148bd09cef7e50110566d8a0 Mon Sep 17 00:00:00 2001
2From: Eric Blake <eblake@redhat.com>
3Date: Thu, 8 Aug 2024 16:05:08 -0500
4Subject: [PATCH] nbd/server: CVE-2024-7409: Drop non-negotiating clients
5
6A client that opens a socket but does not negotiate is merely hogging
7qemu's resources (an open fd and a small amount of memory); and a
8malicious client that can access the port where NBD is listening can
9attempt a denial of service attack by intentionally opening and
10abandoning lots of unfinished connections. The previous patch put a
11default bound on the number of such ongoing connections, but once that
12limit is hit, no more clients can connect (including legitimate ones).
13The solution is to insist that clients complete handshake within a
14reasonable time limit, defaulting to 10 seconds. A client that has
15not successfully completed NBD_OPT_GO by then (including the case of
16where the client didn't know TLS credentials to even reach the point
17of NBD_OPT_GO) is wasting our time and does not deserve to stay
18connected. Later patches will allow fine-tuning the limit away from
19the default value (including disabling it for doing integration
20testing of the handshake process itself).
21
22Note that this patch in isolation actually makes it more likely to see
23qemu SEGV after nbd-server-stop, as any client socket still connected
24when the server shuts down will now be closed after 10 seconds rather
25than at the client's whims. That will be addressed in the next patch.
26
27For a demo of this patch in action:
28$ qemu-nbd -f raw -r -t -e 10 file &
29$ nbdsh --opt-mode -c '
30H = list()
31for i in range(20):
32 print(i)
33 H.insert(i, nbd.NBD())
34 H[i].set_opt_mode(True)
35 H[i].connect_uri("nbd://localhost")
36'
37$ kill $!
38
39where later connections get to start progressing once earlier ones are
40forcefully dropped for taking too long, rather than hanging.
41
42Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
43Signed-off-by: Eric Blake <eblake@redhat.com>
44Message-ID: <20240807174943.771624-13-eblake@redhat.com>
45Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
46[eblake: rebase to changes earlier in series, reduce scope of timer]
47Signed-off-by: Eric Blake <eblake@redhat.com>
48
49CVE: CVE-2024-7409
50Upstream-Status: Backport [https://gitlab.com/qemu-project/qemu/-/commit/b9b72cb3ce15b693148bd09cef7e50110566d8a0]
51Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
52---
53 nbd/server.c | 28 +++++++++++++++++++++++++++-
54 nbd/trace-events | 1 +
55 2 files changed, 28 insertions(+), 1 deletion(-)
56
57diff --git a/nbd/server.c b/nbd/server.c
58index 12680c8dc..1bb253726 100644
59--- a/nbd/server.c
60+++ b/nbd/server.c
61@@ -2698,22 +2698,48 @@ static void nbd_client_receive_next_request(NBDClient *client)
62 }
63 }
64
65+static void nbd_handshake_timer_cb(void *opaque)
66+{
67+ QIOChannel *ioc = opaque;
68+
69+ trace_nbd_handshake_timer_cb();
70+ qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
71+}
72+
73 static coroutine_fn void nbd_co_client_start(void *opaque)
74 {
75 NBDClient *client = opaque;
76 Error *local_err = NULL;
77+ QEMUTimer *handshake_timer = NULL;
78
79 qemu_co_mutex_init(&client->send_lock);
80
81- /* TODO - utilize client->handshake_max_secs */
82+ /*
83+ * Create a timer to bound the time spent in negotiation. If the
84+ * timer expires, it is likely nbd_negotiate will fail because the
85+ * socket was shutdown.
86+ */
87+ if (client->handshake_max_secs > 0) {
88+ handshake_timer = aio_timer_new(qemu_get_aio_context(),
89+ QEMU_CLOCK_REALTIME,
90+ SCALE_NS,
91+ nbd_handshake_timer_cb,
92+ client->sioc);
93+ timer_mod(handshake_timer,
94+ qemu_clock_get_ns(QEMU_CLOCK_REALTIME) +
95+ client->handshake_max_secs * NANOSECONDS_PER_SECOND);
96+ }
97+
98 if (nbd_negotiate(client, &local_err)) {
99 if (local_err) {
100 error_report_err(local_err);
101 }
102+ timer_free(handshake_timer);
103 client_close(client, false);
104 return;
105 }
106
107+ timer_free(handshake_timer);
108 nbd_client_receive_next_request(client);
109 }
110
111diff --git a/nbd/trace-events b/nbd/trace-events
112index c4919a2dd..553546f1f 100644
113--- a/nbd/trace-events
114+++ b/nbd/trace-events
115@@ -73,3 +73,4 @@ nbd_co_receive_request_decode_type(uint64_t handle, uint16_t type, const char *n
116 nbd_co_receive_request_payload_received(uint64_t handle, uint32_t len) "Payload received: handle = %" PRIu64 ", len = %" PRIu32
117 nbd_co_receive_align_compliance(const char *op, uint64_t from, uint32_t len, uint32_t align) "client sent non-compliant unaligned %s request: from=0x%" PRIx64 ", len=0x%" PRIx32 ", align=0x%" PRIx32
118 nbd_trip(void) "Reading request"
119+nbd_handshake_timer_cb(void) "client took too long to negotiate"
120--
1212.25.1
122
diff --git a/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0004.patch b/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0004.patch
new file mode 100644
index 0000000000..ffdb1b0d94
--- /dev/null
+++ b/meta/recipes-devtools/qemu/qemu/CVE-2024-7409-0004.patch
@@ -0,0 +1,163 @@
1From 3e7ef738c8462c45043a1d39f702a0990406a3b3 Mon Sep 17 00:00:00 2001
2From: Eric Blake <eblake@redhat.com>
3Date: Wed, 7 Aug 2024 12:23:13 -0500
4Subject: [PATCH] nbd/server: CVE-2024-7409: Close stray clients at server-stop
5
6A malicious client can attempt to connect to an NBD server, and then
7intentionally delay progress in the handshake, including if it does
8not know the TLS secrets. Although the previous two patches reduce
9this behavior by capping the default max-connections parameter and
10killing slow clients, they did not eliminate the possibility of a
11client waiting to close the socket until after the QMP nbd-server-stop
12command is executed, at which point qemu would SEGV when trying to
13dereference the NULL nbd_server global which is no longer present.
14This amounts to a denial of service attack. Worse, if another NBD
15server is started before the malicious client disconnects, I cannot
16rule out additional adverse effects when the old client interferes
17with the connection count of the new server (although the most likely
18is a crash due to an assertion failure when checking
19nbd_server->connections > 0).
20
21For environments without this patch, the CVE can be mitigated by
22ensuring (such as via a firewall) that only trusted clients can
23connect to an NBD server. Note that using frameworks like libvirt
24that ensure that TLS is used and that nbd-server-stop is not executed
25while any trusted clients are still connected will only help if there
26is also no possibility for an untrusted client to open a connection
27but then stall on the NBD handshake.
28
29Given the previous patches, it would be possible to guarantee that no
30clients remain connected by having nbd-server-stop sleep for longer
31than the default handshake deadline before finally freeing the global
32nbd_server object, but that could make QMP non-responsive for a long
33time. So intead, this patch fixes the problem by tracking all client
34sockets opened while the server is running, and forcefully closing any
35such sockets remaining without a completed handshake at the time of
36nbd-server-stop, then waiting until the coroutines servicing those
37sockets notice the state change. nbd-server-stop now has a second
38AIO_WAIT_WHILE_UNLOCKED (the first is indirectly through the
39blk_exp_close_all_type() that disconnects all clients that completed
40handshakes), but forced socket shutdown is enough to progress the
41coroutines and quickly tear down all clients before the server is
42freed, thus finally fixing the CVE.
43
44This patch relies heavily on the fact that nbd/server.c guarantees
45that it only calls nbd_blockdev_client_closed() from the main loop
46(see the assertion in nbd_client_put() and the hoops used in
47nbd_client_put_nonzero() to achieve that); if we did not have that
48guarantee, we would also need a mutex protecting our accesses of the
49list of connections to survive re-entrancy from independent iothreads.
50
51Although I did not actually try to test old builds, it looks like this
52problem has existed since at least commit 862172f45c (v2.12.0, 2017) -
53even back when that patch started using a QIONetListener to handle
54listening on multiple sockets, nbd_server_free() was already unaware
55that the nbd_blockdev_client_closed callback can be reached later by a
56client thread that has not completed handshakes (and therefore the
57client's socket never got added to the list closed in
58nbd_export_close_all), despite that patch intentionally tearing down
59the QIONetListener to prevent new clients.
60
61Reported-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
62Fixes: CVE-2024-7409
63CC: qemu-stable@nongnu.org
64Signed-off-by: Eric Blake <eblake@redhat.com>
65Message-ID: <20240807174943.771624-14-eblake@redhat.com>
66Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
67
68CVE: CVE-2024-7409
69Upstream-Status: Backport [https://gitlab.com/qemu-project/qemu/-/commit/3e7ef738c8462c45043a1d39f702a0990406a3b3]
70Signed-off-by: Hitendra Prajapati <hprajapati@mvista.com>
71---
72 blockdev-nbd.c | 35 ++++++++++++++++++++++++++++++++++-
73 1 file changed, 34 insertions(+), 1 deletion(-)
74
75diff --git a/blockdev-nbd.c b/blockdev-nbd.c
76index 4bd90bac1..c71ca38d2 100644
77--- a/blockdev-nbd.c
78+++ b/blockdev-nbd.c
79@@ -21,12 +21,18 @@
80 #include "io/channel-socket.h"
81 #include "io/net-listener.h"
82
83+typedef struct NBDConn {
84+ QIOChannelSocket *cioc;
85+ QLIST_ENTRY(NBDConn) next;
86+} NBDConn;
87+
88 typedef struct NBDServerData {
89 QIONetListener *listener;
90 QCryptoTLSCreds *tlscreds;
91 char *tlsauthz;
92 uint32_t max_connections;
93 uint32_t connections;
94+ QLIST_HEAD(, NBDConn) conns;
95 } NBDServerData;
96
97 static NBDServerData *nbd_server;
98@@ -46,6 +52,14 @@ bool nbd_server_is_running(void)
99
100 static void nbd_blockdev_client_closed(NBDClient *client, bool ignored)
101 {
102+ NBDConn *conn = nbd_client_owner(client);
103+
104+ assert(qemu_mutex_iothread_locked() && nbd_server);
105+
106+ object_unref(OBJECT(conn->cioc));
107+ QLIST_REMOVE(conn, next);
108+ g_free(conn);
109+
110 nbd_client_put(client);
111 assert(nbd_server->connections > 0);
112 nbd_server->connections--;
113@@ -55,14 +69,20 @@ static void nbd_blockdev_client_closed(NBDClient *client, bool ignored)
114 static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
115 gpointer opaque)
116 {
117+ NBDConn *conn = g_new0(NBDConn, 1);
118+
119+ assert(qemu_mutex_iothread_locked() && nbd_server);
120 nbd_server->connections++;
121+ object_ref(OBJECT(cioc));
122+ conn->cioc = cioc;
123+ QLIST_INSERT_HEAD(&nbd_server->conns, conn, next);
124 nbd_update_server_watch(nbd_server);
125
126 qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server");
127 /* TODO - expose handshake timeout as QMP option */
128 nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_MAX_SECS,
129 nbd_server->tlscreds, nbd_server->tlsauthz,
130- nbd_blockdev_client_closed, NULL);
131+ nbd_blockdev_client_closed, conn);
132 }
133
134 static void nbd_update_server_watch(NBDServerData *s)
135@@ -76,12 +96,25 @@ static void nbd_update_server_watch(NBDServerData *s)
136
137 static void nbd_server_free(NBDServerData *server)
138 {
139+ NBDConn *conn, *tmp;
140+
141 if (!server) {
142 return;
143 }
144
145+ /*
146+ * Forcefully close the listener socket, and any clients that have
147+ * not yet disconnected on their own.
148+ */
149 qio_net_listener_disconnect(server->listener);
150 object_unref(OBJECT(server->listener));
151+ QLIST_FOREACH_SAFE(conn, &server->conns, next, tmp) {
152+ qio_channel_shutdown(QIO_CHANNEL(conn->cioc), QIO_CHANNEL_SHUTDOWN_BOTH,
153+ NULL);
154+ }
155+
156+ AIO_WAIT_WHILE(NULL, server->connections > 0);
157+
158 if (server->tlscreds) {
159 object_unref(OBJECT(server->tlscreds));
160 }
161--
1622.25.1
163