Import from CS git

This commit is contained in:
eabdullin 2024-08-27 15:10:21 +00:00
parent e5b0526845
commit 8fd0a7fee9
12 changed files with 1373 additions and 1 deletions

View File

@ -0,0 +1,61 @@
From f4623ea611a74c684b0097b98a803cbe7ffb0825 Mon Sep 17 00:00:00 2001
From: Jon Maloy <jmaloy@redhat.com>
Date: Thu, 18 Jul 2024 09:26:55 -0400
Subject: [PATCH 5/6] hw/char/virtio-serial-bus: Protect from DMA re-entrancy
bugs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
RH-Author: Jon Maloy <jmaloy@redhat.com>
RH-MergeRequest: 380: QEMU: virtio: DMA reentrancy issue leads to double free vulnerability
RH-Jira: RHEL-32276
RH-Acked-by: Gerd Hoffmann <None>
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
RH-Commit: [5/6] fc8a445ebf6e763cd1482cd1f7ee23e5b5bbb388 (redhat/rhel/src/qemu-kvm/jons-qemu-kvm-2)
JIRA: https://issues.redhat.com/browse/RHEL-32276
CVE: CVE-2024-3446
Upstream: Merged
commit b4295bff25f7b50de1d9cc94a9c6effd40056bca
Author: Philippe Mathieu-Daudé <philmd@linaro.org>
Date: Thu Apr 4 20:56:35 2024 +0200
hw/char/virtio-serial-bus: Protect from DMA re-entrancy bugs
Replace qemu_bh_new_guarded() by virtio_bh_new_guarded()
so the bus and device use the same guard. Otherwise the
DMA-reentrancy protection can be bypassed.
Fixes: CVE-2024-3446
Cc: qemu-stable@nongnu.org
Suggested-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20240409105537.18308-4-philmd@linaro.org>
Signed-off-by: Jon Maloy <jmaloy@redhat.com>
---
hw/char/virtio-serial-bus.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index f18124b155..791b7ac59e 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -985,8 +985,7 @@ static void virtser_port_device_realize(DeviceState *dev, Error **errp)
return;
}
- port->bh = qemu_bh_new_guarded(flush_queued_data_bh, port,
- &dev->mem_reentrancy_guard);
+ port->bh = virtio_bh_new_guarded(dev, flush_queued_data_bh, port);
port->elem = NULL;
}
--
2.39.3

View File

@ -0,0 +1,160 @@
From d37035373a266644b241aab1f041ab09c9185540 Mon Sep 17 00:00:00 2001
From: Jon Maloy <jmaloy@redhat.com>
Date: Thu, 18 Jul 2024 09:29:54 -0400
Subject: [PATCH 4/6] hw/display/virtio-gpu: Protect from DMA re-entrancy bugs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
RH-Author: Jon Maloy <jmaloy@redhat.com>
RH-MergeRequest: 380: QEMU: virtio: DMA reentrancy issue leads to double free vulnerability
RH-Jira: RHEL-32276
RH-Acked-by: Gerd Hoffmann <None>
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
RH-Commit: [4/6] e3cd21742228528a1a74ea62d55b5941d3efb261 (redhat/rhel/src/qemu-kvm/jons-qemu-kvm-2)
JIRA: https://issues.redhat.com/browse/RHEL-32276
CVE: CVE-2024-3446
Upstream: Merged
commit ba28e0ff4d95b56dc334aac2730ab3651ffc3132
Author: Philippe Mathieu-Daudé <philmd@linaro.org>
Date: Thu Apr 4 20:56:27 2024 +0200
hw/display/virtio-gpu: Protect from DMA re-entrancy bugs
Replace qemu_bh_new_guarded() by virtio_bh_new_guarded()
so the bus and device use the same guard. Otherwise the
DMA-reentrancy protection can be bypassed:
$ cat << EOF | qemu-system-i386 -display none -nodefaults \
-machine q35,accel=qtest \
-m 512M \
-device virtio-gpu \
-qtest stdio
outl 0xcf8 0x80000820
outl 0xcfc 0xe0004000
outl 0xcf8 0x80000804
outw 0xcfc 0x06
write 0xe0004030 0x4 0x024000e0
write 0xe0004028 0x1 0xff
write 0xe0004020 0x4 0x00009300
write 0xe000401c 0x1 0x01
write 0x101 0x1 0x04
write 0x103 0x1 0x1c
write 0x9301c8 0x1 0x18
write 0x105 0x1 0x1c
write 0x107 0x1 0x1c
write 0x109 0x1 0x1c
write 0x10b 0x1 0x00
write 0x10d 0x1 0x00
write 0x10f 0x1 0x00
write 0x111 0x1 0x00
write 0x113 0x1 0x00
write 0x115 0x1 0x00
write 0x117 0x1 0x00
write 0x119 0x1 0x00
write 0x11b 0x1 0x00
write 0x11d 0x1 0x00
write 0x11f 0x1 0x00
write 0x121 0x1 0x00
write 0x123 0x1 0x00
write 0x125 0x1 0x00
write 0x127 0x1 0x00
write 0x129 0x1 0x00
write 0x12b 0x1 0x00
write 0x12d 0x1 0x00
write 0x12f 0x1 0x00
write 0x131 0x1 0x00
write 0x133 0x1 0x00
write 0x135 0x1 0x00
write 0x137 0x1 0x00
write 0x139 0x1 0x00
write 0xe0007003 0x1 0x00
EOF
...
=================================================================
==276099==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d000011178
at pc 0x562cc3b736c7 bp 0x7ffed49dee60 sp 0x7ffed49dee58
READ of size 8 at 0x60d000011178 thread T0
#0 0x562cc3b736c6 in virtio_gpu_ctrl_response hw/display/virtio-gpu.c:180:42
#1 0x562cc3b7c40b in virtio_gpu_ctrl_response_nodata hw/display/virtio-gpu.c:192:5
#2 0x562cc3b7c40b in virtio_gpu_simple_process_cmd hw/display/virtio-gpu.c:1015:13
#3 0x562cc3b82873 in virtio_gpu_process_cmdq hw/display/virtio-gpu.c:1050:9
#4 0x562cc4a85514 in aio_bh_call util/async.c:169:5
#5 0x562cc4a85c52 in aio_bh_poll util/async.c:216:13
#6 0x562cc4a1a79b in aio_dispatch util/aio-posix.c:423:5
#7 0x562cc4a8a2da in aio_ctx_dispatch util/async.c:358:5
#8 0x7f36840547a8 in g_main_context_dispatch (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x547a8)
#9 0x562cc4a8b753 in glib_pollfds_poll util/main-loop.c:290:9
#10 0x562cc4a8b753 in os_host_main_loop_wait util/main-loop.c:313:5
#11 0x562cc4a8b753 in main_loop_wait util/main-loop.c:592:11
#12 0x562cc3938186 in qemu_main_loop system/runstate.c:782:9
#13 0x562cc43b7af5 in qemu_default_main system/main.c:37:14
#14 0x7f3683a6c189 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#15 0x7f3683a6c244 in __libc_start_main csu/../csu/libc-start.c:381:3
#16 0x562cc2a58ac0 in _start (qemu-system-i386+0x231bac0)
0x60d000011178 is located 56 bytes inside of 136-byte region [0x60d000011140,0x60d0000111c8)
freed by thread T0 here:
#0 0x562cc2adb662 in __interceptor_free (qemu-system-i386+0x239e662)
#1 0x562cc3b86b21 in virtio_gpu_reset hw/display/virtio-gpu.c:1524:9
#2 0x562cc416e20e in virtio_reset hw/virtio/virtio.c:2145:9
#3 0x562cc37c5644 in virtio_pci_reset hw/virtio/virtio-pci.c:2249:5
#4 0x562cc4233758 in memory_region_write_accessor system/memory.c:497:5
#5 0x562cc4232eea in access_with_adjusted_size system/memory.c:573:18
previously allocated by thread T0 here:
#0 0x562cc2adb90e in malloc (qemu-system-i386+0x239e90e)
#1 0x7f368405a678 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5a678)
#2 0x562cc4163ffc in virtqueue_split_pop hw/virtio/virtio.c:1612:12
#3 0x562cc4163ffc in virtqueue_pop hw/virtio/virtio.c:1783:16
#4 0x562cc3b91a95 in virtio_gpu_handle_ctrl hw/display/virtio-gpu.c:1112:15
#5 0x562cc4a85514 in aio_bh_call util/async.c:169:5
#6 0x562cc4a85c52 in aio_bh_poll util/async.c:216:13
#7 0x562cc4a1a79b in aio_dispatch util/aio-posix.c:423:5
SUMMARY: AddressSanitizer: heap-use-after-free hw/display/virtio-gpu.c:180:42 in virtio_gpu_ctrl_response
With this change, the same reproducer triggers:
qemu-system-i386: warning: Blocked re-entrant IO on MemoryRegion: virtio-pci-common-virtio-gpu at addr: 0x6
Fixes: CVE-2024-3446
Cc: qemu-stable@nongnu.org
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Reported-by: Yongkang Jia <kangel@zju.edu.cn>
Reported-by: Xiao Lei <nop.leixiao@gmail.com>
Reported-by: Yiming Tao <taoym@zju.edu.cn>
Buglink: https://bugs.launchpad.net/qemu/+bug/1888606
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20240409105537.18308-3-philmd@linaro.org>
Signed-off-by: Jon Maloy <jmaloy@redhat.com>
---
hw/display/virtio-gpu.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index c28ce1ea72..64fdc18478 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1334,10 +1334,8 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
g->ctrl_vq = virtio_get_queue(vdev, 0);
g->cursor_vq = virtio_get_queue(vdev, 1);
- g->ctrl_bh = qemu_bh_new_guarded(virtio_gpu_ctrl_bh, g,
- &qdev->mem_reentrancy_guard);
- g->cursor_bh = qemu_bh_new_guarded(virtio_gpu_cursor_bh, g,
- &qdev->mem_reentrancy_guard);
+ g->ctrl_bh = virtio_bh_new_guarded(qdev, virtio_gpu_ctrl_bh, g);
+ g->cursor_bh = virtio_bh_new_guarded(qdev, virtio_gpu_cursor_bh, g);
g->reset_bh = qemu_bh_new(virtio_gpu_reset_bh, g);
qemu_cond_init(&g->reset_cond);
QTAILQ_INIT(&g->reslist);
--
2.39.3

View File

@ -0,0 +1,86 @@
From 1b62d61c495bf4cd3a819ab8d1ef024d153e0ece Mon Sep 17 00:00:00 2001
From: Jon Maloy <jmaloy@redhat.com>
Date: Thu, 18 Jul 2024 09:40:29 -0400
Subject: [PATCH 3/6] hw/virtio: Introduce virtio_bh_new_guarded() helper
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
RH-Author: Jon Maloy <jmaloy@redhat.com>
RH-MergeRequest: 380: QEMU: virtio: DMA reentrancy issue leads to double free vulnerability
RH-Jira: RHEL-32276
RH-Acked-by: Gerd Hoffmann <None>
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
RH-Commit: [3/6] 1cbde7ddb8393b72e2e8d457b5e2d739116567a9 (redhat/rhel/src/qemu-kvm/jons-qemu-kvm-2)
JIRA: https://issues.redhat.com/browse/RHEL-32276
CVE: CVE-2024-3446
Upstream: Merged
commit ec0504b989ca61e03636384d3602b7bf07ffe4da
Author: Philippe Mathieu-Daudé <philmd@linaro.org>
Date: Thu Apr 4 20:56:11 2024 +0200
hw/virtio: Introduce virtio_bh_new_guarded() helper
Introduce virtio_bh_new_guarded(), similar to qemu_bh_new_guarded()
but using the transport memory guard, instead of the device one
(there can only be one virtio device per virtio bus).
Inspired-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20240409105537.18308-2-philmd@linaro.org>
Signed-off-by: Jon Maloy <jmaloy@redhat.com>
---
hw/virtio/virtio.c | 10 ++++++++++
include/hw/virtio/virtio.h | 7 +++++++
2 files changed, 17 insertions(+)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index ea7c079fb0..5ae9c44841 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3874,3 +3874,13 @@ static void virtio_register_types(void)
}
type_init(virtio_register_types)
+
+QEMUBH *virtio_bh_new_guarded_full(DeviceState *dev,
+ QEMUBHFunc *cb, void *opaque,
+ const char *name)
+{
+ DeviceState *transport = qdev_get_parent_bus(dev)->parent;
+
+ return qemu_bh_new_full(cb, opaque, name,
+ &transport->mem_reentrancy_guard);
+}
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 8bab9cfb75..731c631a81 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -22,6 +22,7 @@
#include "standard-headers/linux/virtio_config.h"
#include "standard-headers/linux/virtio_ring.h"
#include "qom/object.h"
+#include "block/aio.h"
/* A guest should never accept this. It implies negotiation is broken. */
#define VIRTIO_F_BAD_FEATURE 30
@@ -397,4 +398,10 @@ static inline bool virtio_device_disabled(VirtIODevice *vdev)
bool virtio_legacy_allowed(VirtIODevice *vdev);
bool virtio_legacy_check_disabled(VirtIODevice *vdev);
+QEMUBH *virtio_bh_new_guarded_full(DeviceState *dev,
+ QEMUBHFunc *cb, void *opaque,
+ const char *name);
+#define virtio_bh_new_guarded(dev, cb, opaque) \
+ virtio_bh_new_guarded_full((dev), (cb), (opaque), (stringify(cb)))
+
#endif
--
2.39.3

View File

@ -0,0 +1,62 @@
From 2ecbd673a0e2191821ce88128587f709936ad765 Mon Sep 17 00:00:00 2001
From: Jon Maloy <jmaloy@redhat.com>
Date: Thu, 18 Jul 2024 09:21:27 -0400
Subject: [PATCH 6/6] hw/virtio/virtio-crypto: Protect from DMA re-entrancy
bugs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
RH-Author: Jon Maloy <jmaloy@redhat.com>
RH-MergeRequest: 380: QEMU: virtio: DMA reentrancy issue leads to double free vulnerability
RH-Jira: RHEL-32276
RH-Acked-by: Gerd Hoffmann <None>
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
RH-Commit: [6/6] 975ac4640fd8e7cbf3820757787ee7b1270173be (redhat/rhel/src/qemu-kvm/jons-qemu-kvm-2)
JIRA: https://issues.redhat.com/browse/RHEL-32276
CVE: CVE-2024-3446
Upstream: Merged
commit f4729ec39ad97a42ceaa7b5697f84f440ea6e5dc
Author: Philippe Mathieu-Daudé <philmd@linaro.org>
Date: Thu Apr 4 20:56:41 2024 +0200
hw/virtio/virtio-crypto: Protect from DMA re-entrancy bugs
Replace qemu_bh_new_guarded() by virtio_bh_new_guarded()
so the bus and device use the same guard. Otherwise the
DMA-reentrancy protection can be bypassed.
Fixes: CVE-2024-3446
Cc: qemu-stable@nongnu.org
Suggested-by: Alexander Bulekov <alxndr@bu.edu>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20240409105537.18308-5-philmd@linaro.org>
Signed-off-by: Jon Maloy <jmaloy@redhat.com>
---
hw/virtio/virtio-crypto.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c
index 1be7bb543c..1741d4aba1 100644
--- a/hw/virtio/virtio-crypto.c
+++ b/hw/virtio/virtio-crypto.c
@@ -817,8 +817,8 @@ static void virtio_crypto_device_realize(DeviceState *dev, Error **errp)
vcrypto->vqs[i].dataq =
virtio_add_queue(vdev, 1024, virtio_crypto_handle_dataq_bh);
vcrypto->vqs[i].dataq_bh =
- qemu_bh_new_guarded(virtio_crypto_dataq_bh, &vcrypto->vqs[i],
- &dev->mem_reentrancy_guard);
+ virtio_bh_new_guarded(dev, virtio_crypto_dataq_bh,
+ &vcrypto->vqs[i]);
vcrypto->vqs[i].vcrypto = vcrypto;
}
--
2.39.3

View File

@ -0,0 +1,187 @@
From adfddc25c82576458442f61efb913e44d83bcbd0 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Tue, 6 Aug 2024 13:53:00 -0500
Subject: [PATCH 2/5] nbd/server: CVE-2024-7409: Cap default max-connections to
100
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
RH-Author: Eric Blake <eblake@redhat.com>
RH-MergeRequest: 388: nbd/server: fix CVE-2024-7409 (qemu crash on nbd-server-stop) [rhel-8.10.z]
RH-Jira: RHEL-52611
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
RH-Acked-by: Richard W.M. Jones <rjones@redhat.com>
RH-Commit: [2/4] 1f5d88d5644c46cbb957778254a993930b9d86dc (ebblake/qemu-kvm)
Allowing an unlimited number of clients to any web service is a recipe
for a rudimentary denial of service attack: the client merely needs to
open lots of sockets without closing them, until qemu no longer has
any more fds available to allocate.
For qemu-nbd, we default to allowing only 1 connection unless more are
explicitly asked for (-e or --shared); this was historically picked as
a nice default (without an explicit -t, a non-persistent qemu-nbd goes
away after a client disconnects, without needing any additional
follow-up commands), and we are not going to change that interface now
(besides, someday we want to point people towards qemu-storage-daemon
instead of qemu-nbd).
But for qemu proper, and the newer qemu-storage-daemon, the QMP
nbd-server-start command has historically had a default of unlimited
number of connections, in part because unlike qemu-nbd it is
inherently persistent until nbd-server-stop. Allowing multiple client
sockets is particularly useful for clients that can take advantage of
MULTI_CONN (creating parallel sockets to increase throughput),
although known clients that do so (such as libnbd's nbdcopy) typically
use only 8 or 16 connections (the benefits of scaling diminish once
more sockets are competing for kernel attention). Picking a number
large enough for typical use cases, but not unlimited, makes it
slightly harder for a malicious client to perform a denial of service
merely by opening lots of connections withot progressing through the
handshake.
This change does not eliminate CVE-2024-7409 on its own, but reduces
the chance for fd exhaustion or unlimited memory usage as an attack
surface. On the other hand, by itself, it makes it more obvious that
with a finite limit, we have the problem of an unauthenticated client
holding 100 fds opened as a way to block out a legitimate client from
being able to connect; thus, later patches will further add timeouts
to reject clients that are not making progress.
This is an INTENTIONAL change in behavior, and will break any client
of nbd-server-start that was not passing an explicit max-connections
parameter, yet expects more than 100 simultaneous connections. We are
not aware of any such client (as stated above, most clients aware of
MULTI_CONN get by just fine on 8 or 16 connections, and probably cope
with later connections failing by relying on the earlier connections;
libvirt has not yet been passing max-connections, but generally
creates NBD servers with the intent for a single client for the sake
of live storage migration; meanwhile, the KubeSAN project anticipates
a large cluster sharing multiple clients [up to 8 per node, and up to
100 nodes in a cluster], but it currently uses qemu-nbd with an
explicit --shared=0 rather than qemu-storage-daemon with
nbd-server-start).
We considered using a deprecation period (declare that omitting
max-parameters is deprecated, and make it mandatory in 3 releases -
then we don't need to pick an arbitrary default); that has zero risk
of breaking any apps that accidentally depended on more than 100
connections, and where such breakage might not be noticed under unit
testing but only under the larger loads of production usage. But it
does not close the denial-of-service hole until far into the future,
and requires all apps to change to add the parameter even if 100 was
good enough. It also has a drawback that any app (like libvirt) that
is accidentally relying on an unlimited default should seriously
consider their own CVE now, at which point they are going to change to
pass explicit max-connections sooner than waiting for 3 qemu releases.
Finally, if our changed default breaks an app, that app can always
pass in an explicit max-parameters with a larger value.
It is also intentional that the HMP interface to nbd-server-start is
not changed to expose max-connections (any client needing to fine-tune
things should be using QMP).
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20240807174943.771624-12-eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
[ericb: Expand commit message to summarize Dan's argument for why we
break corner-case back-compat behavior without a deprecation period]
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit c8a76dbd90c2f48df89b75bef74917f90a59b623)
Conflicts:
qapi/block-export.json - context (no multi-conn, older format)
Jira: https://issues.redhat.com/browse/RHEL-52611
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/monitor/block-hmp-cmds.c | 3 ++-
blockdev-nbd.c | 8 ++++++++
include/block/nbd.h | 7 +++++++
qapi/block-export.json | 4 ++--
4 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index 2ac4aedfff..32a666b5dc 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -411,7 +411,8 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
goto exit;
}
- nbd_server_start(addr, NULL, NULL, 0, &local_err);
+ nbd_server_start(addr, NULL, NULL, NBD_DEFAULT_MAX_CONNECTIONS,
+ &local_err);
qapi_free_SocketAddress(addr);
if (local_err != NULL) {
goto exit;
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index b9e8dc78f3..4bd90bac16 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -171,6 +171,10 @@ void nbd_server_start(SocketAddress *addr, const char *tls_creds,
void nbd_server_start_options(NbdServerOptions *arg, Error **errp)
{
+ if (!arg->has_max_connections) {
+ arg->max_connections = NBD_DEFAULT_MAX_CONNECTIONS;
+ }
+
nbd_server_start(arg->addr, arg->tls_creds, arg->tls_authz,
arg->max_connections, errp);
}
@@ -183,6 +187,10 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr,
{
SocketAddress *addr_flat = socket_address_flatten(addr);
+ if (!has_max_connections) {
+ max_connections = NBD_DEFAULT_MAX_CONNECTIONS;
+ }
+
nbd_server_start(addr_flat, tls_creds, tls_authz, max_connections, errp);
qapi_free_SocketAddress(addr_flat);
}
diff --git a/include/block/nbd.h b/include/block/nbd.h
index b71a297249..a31c34a8a6 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -33,6 +33,13 @@ extern const BlockExportDriver blk_exp_nbd;
*/
#define NBD_DEFAULT_HANDSHAKE_MAX_SECS 10
+/*
+ * NBD_DEFAULT_MAX_CONNECTIONS: Number of client sockets to allow at
+ * once; must be large enough to allow a MULTI_CONN-aware client like
+ * nbdcopy to create its typical number of 8-16 sockets.
+ */
+#define NBD_DEFAULT_MAX_CONNECTIONS 100
+
/* Handshake phase structs - this struct is passed on the wire */
struct NBDOption {
diff --git a/qapi/block-export.json b/qapi/block-export.json
index c1b92ce1c1..181d7238fe 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -21,7 +21,7 @@
# recreated on the fly while the NBD server is active.
# If missing, it will default to denying access (since 4.0).
# @max-connections: The maximum number of connections to allow at the same
-# time, 0 for unlimited. (since 5.2; default: 0)
+# time, 0 for unlimited. (since 5.2; default: 100)
#
# Since: 4.2
##
@@ -50,7 +50,7 @@
# recreated on the fly while the NBD server is active.
# If missing, it will default to denying access (since 4.0).
# @max-connections: The maximum number of connections to allow at the same
-# time, 0 for unlimited. (since 5.2; default: 0)
+# time, 0 for unlimited. (since 5.2; default: 100)
#
# Returns: error if the server is already running.
#
--
2.39.3

View File

@ -0,0 +1,180 @@
From 4ab086cdf9a5842c49f3fe59baff1747d863b97a Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 7 Aug 2024 12:23:13 -0500
Subject: [PATCH 4/5] nbd/server: CVE-2024-7409: Close stray clients at
server-stop
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
RH-Author: Eric Blake <eblake@redhat.com>
RH-MergeRequest: 388: nbd/server: fix CVE-2024-7409 (qemu crash on nbd-server-stop) [rhel-8.10.z]
RH-Jira: RHEL-52611
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
RH-Acked-by: Richard W.M. Jones <rjones@redhat.com>
RH-Commit: [4/4] 92a20764dbee3cf94181cab412d90cbf92b4a417 (ebblake/qemu-kvm)
A malicious client can attempt to connect to an NBD server, and then
intentionally delay progress in the handshake, including if it does
not know the TLS secrets. Although the previous two patches reduce
this behavior by capping the default max-connections parameter and
killing slow clients, they did not eliminate the possibility of a
client waiting to close the socket until after the QMP nbd-server-stop
command is executed, at which point qemu would SEGV when trying to
dereference the NULL nbd_server global which is no longer present.
This amounts to a denial of service attack. Worse, if another NBD
server is started before the malicious client disconnects, I cannot
rule out additional adverse effects when the old client interferes
with the connection count of the new server (although the most likely
is a crash due to an assertion failure when checking
nbd_server->connections > 0).
For environments without this patch, the CVE can be mitigated by
ensuring (such as via a firewall) that only trusted clients can
connect to an NBD server. Note that using frameworks like libvirt
that ensure that TLS is used and that nbd-server-stop is not executed
while any trusted clients are still connected will only help if there
is also no possibility for an untrusted client to open a connection
but then stall on the NBD handshake.
Given the previous patches, it would be possible to guarantee that no
clients remain connected by having nbd-server-stop sleep for longer
than the default handshake deadline before finally freeing the global
nbd_server object, but that could make QMP non-responsive for a long
time. So intead, this patch fixes the problem by tracking all client
sockets opened while the server is running, and forcefully closing any
such sockets remaining without a completed handshake at the time of
nbd-server-stop, then waiting until the coroutines servicing those
sockets notice the state change. nbd-server-stop now has a second
AIO_WAIT_WHILE_UNLOCKED (the first is indirectly through the
blk_exp_close_all_type() that disconnects all clients that completed
handshakes), but forced socket shutdown is enough to progress the
coroutines and quickly tear down all clients before the server is
freed, thus finally fixing the CVE.
This patch relies heavily on the fact that nbd/server.c guarantees
that it only calls nbd_blockdev_client_closed() from the main loop
(see the assertion in nbd_client_put() and the hoops used in
nbd_client_put_nonzero() to achieve that); if we did not have that
guarantee, we would also need a mutex protecting our accesses of the
list of connections to survive re-entrancy from independent iothreads.
Although I did not actually try to test old builds, it looks like this
problem has existed since at least commit 862172f45c (v2.12.0, 2017) -
even back when that patch started using a QIONetListener to handle
listening on multiple sockets, nbd_server_free() was already unaware
that the nbd_blockdev_client_closed callback can be reached later by a
client thread that has not completed handshakes (and therefore the
client's socket never got added to the list closed in
nbd_export_close_all), despite that patch intentionally tearing down
the QIONetListener to prevent new clients.
Reported-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
Fixes: CVE-2024-7409
CC: qemu-stable@nongnu.org
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20240807174943.771624-14-eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 3e7ef738c8462c45043a1d39f702a0990406a3b3)
Conflicts:
- blockdev-nbd.c:
- qemu_in_main_thread() not backported, but only used in assertions so
safe to drop
- AIO_WAIT_WHILE_UNLOCKED() not backported, use AIO_WAIT_WHILE() like
blk_exp_close_all_type()
Jira: https://issues.redhat.com/browse/RHEL-52611
Signed-off-by: Eric Blake <eblake@redhat.com>
---
blockdev-nbd.c | 35 ++++++++++++++++++++++++++++++++++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 4bd90bac16..87839c180b 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -21,12 +21,18 @@
#include "io/channel-socket.h"
#include "io/net-listener.h"
+typedef struct NBDConn {
+ QIOChannelSocket *cioc;
+ QLIST_ENTRY(NBDConn) next;
+} NBDConn;
+
typedef struct NBDServerData {
QIONetListener *listener;
QCryptoTLSCreds *tlscreds;
char *tlsauthz;
uint32_t max_connections;
uint32_t connections;
+ QLIST_HEAD(, NBDConn) conns;
} NBDServerData;
static NBDServerData *nbd_server;
@@ -46,6 +52,14 @@ bool nbd_server_is_running(void)
static void nbd_blockdev_client_closed(NBDClient *client, bool ignored)
{
+ NBDConn *conn = nbd_client_owner(client);
+
+ assert(nbd_server);
+
+ object_unref(OBJECT(conn->cioc));
+ QLIST_REMOVE(conn, next);
+ g_free(conn);
+
nbd_client_put(client);
assert(nbd_server->connections > 0);
nbd_server->connections--;
@@ -55,14 +69,20 @@ static void nbd_blockdev_client_closed(NBDClient *client, bool ignored)
static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
gpointer opaque)
{
+ NBDConn *conn = g_new0(NBDConn, 1);
+
+ assert(nbd_server);
nbd_server->connections++;
+ object_ref(OBJECT(cioc));
+ conn->cioc = cioc;
+ QLIST_INSERT_HEAD(&nbd_server->conns, conn, next);
nbd_update_server_watch(nbd_server);
qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server");
/* TODO - expose handshake timeout as QMP option */
nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_MAX_SECS,
nbd_server->tlscreds, nbd_server->tlsauthz,
- nbd_blockdev_client_closed, NULL);
+ nbd_blockdev_client_closed, conn);
}
static void nbd_update_server_watch(NBDServerData *s)
@@ -76,12 +96,25 @@ static void nbd_update_server_watch(NBDServerData *s)
static void nbd_server_free(NBDServerData *server)
{
+ NBDConn *conn, *tmp;
+
if (!server) {
return;
}
+ /*
+ * Forcefully close the listener socket, and any clients that have
+ * not yet disconnected on their own.
+ */
qio_net_listener_disconnect(server->listener);
object_unref(OBJECT(server->listener));
+ QLIST_FOREACH_SAFE(conn, &server->conns, next, tmp) {
+ qio_channel_shutdown(QIO_CHANNEL(conn->cioc), QIO_CHANNEL_SHUTDOWN_BOTH,
+ NULL);
+ }
+
+ AIO_WAIT_WHILE(NULL, server->connections > 0);
+
if (server->tlscreds) {
object_unref(OBJECT(server->tlscreds));
}
--
2.39.3

View File

@ -0,0 +1,135 @@
From faac5261d5a9af155950c4e7779c5a4721562824 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Thu, 8 Aug 2024 16:05:08 -0500
Subject: [PATCH 3/5] nbd/server: CVE-2024-7409: Drop non-negotiating clients
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
RH-Author: Eric Blake <eblake@redhat.com>
RH-MergeRequest: 388: nbd/server: fix CVE-2024-7409 (qemu crash on nbd-server-stop) [rhel-8.10.z]
RH-Jira: RHEL-52611
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
RH-Acked-by: Richard W.M. Jones <rjones@redhat.com>
RH-Commit: [3/4] 8c39829f8efbded9af018a4b915af266a55a793a (ebblake/qemu-kvm)
A client that opens a socket but does not negotiate is merely hogging
qemu's resources (an open fd and a small amount of memory); and a
malicious client that can access the port where NBD is listening can
attempt a denial of service attack by intentionally opening and
abandoning lots of unfinished connections. The previous patch put a
default bound on the number of such ongoing connections, but once that
limit is hit, no more clients can connect (including legitimate ones).
The solution is to insist that clients complete handshake within a
reasonable time limit, defaulting to 10 seconds. A client that has
not successfully completed NBD_OPT_GO by then (including the case of
where the client didn't know TLS credentials to even reach the point
of NBD_OPT_GO) is wasting our time and does not deserve to stay
connected. Later patches will allow fine-tuning the limit away from
the default value (including disabling it for doing integration
testing of the handshake process itself).
Note that this patch in isolation actually makes it more likely to see
qemu SEGV after nbd-server-stop, as any client socket still connected
when the server shuts down will now be closed after 10 seconds rather
than at the client's whims. That will be addressed in the next patch.
For a demo of this patch in action:
$ qemu-nbd -f raw -r -t -e 10 file &
$ nbdsh --opt-mode -c '
H = list()
for i in range(20):
print(i)
H.insert(i, nbd.NBD())
H[i].set_opt_mode(True)
H[i].connect_uri("nbd://localhost")
'
$ kill $!
where later connections get to start progressing once earlier ones are
forcefully dropped for taking too long, rather than hanging.
Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20240807174943.771624-13-eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
[eblake: rebase to changes earlier in series, reduce scope of timer]
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit b9b72cb3ce15b693148bd09cef7e50110566d8a0)
Conflicts:
nbd/server.c - context with different aiocontext locking
nbd/trace-events - context with no client-connection.c
Jira: https://issues.redhat.com/browse/RHEL-52611
Signed-off-by: Eric Blake <eblake@redhat.com>
---
nbd/server.c | 28 +++++++++++++++++++++++++++-
nbd/trace-events | 1 +
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/nbd/server.c b/nbd/server.c
index cc1b6838bf..1265068f70 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2701,22 +2701,48 @@ static void nbd_client_receive_next_request(NBDClient *client)
}
}
+static void nbd_handshake_timer_cb(void *opaque)
+{
+ QIOChannel *ioc = opaque;
+
+ trace_nbd_handshake_timer_cb();
+ qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
+}
+
static coroutine_fn void nbd_co_client_start(void *opaque)
{
NBDClient *client = opaque;
Error *local_err = NULL;
+ QEMUTimer *handshake_timer = NULL;
qemu_co_mutex_init(&client->send_lock);
- /* TODO - utilize client->handshake_max_secs */
+ /*
+ * Create a timer to bound the time spent in negotiation. If the
+ * timer expires, it is likely nbd_negotiate will fail because the
+ * socket was shutdown.
+ */
+ if (client->handshake_max_secs > 0) {
+ handshake_timer = aio_timer_new(qemu_get_aio_context(),
+ QEMU_CLOCK_REALTIME,
+ SCALE_NS,
+ nbd_handshake_timer_cb,
+ client->sioc);
+ timer_mod(handshake_timer,
+ qemu_clock_get_ns(QEMU_CLOCK_REALTIME) +
+ client->handshake_max_secs * NANOSECONDS_PER_SECOND);
+ }
+
if (nbd_negotiate(client, &local_err)) {
if (local_err) {
error_report_err(local_err);
}
+ timer_free(handshake_timer);
client_close(client, false);
return;
}
+ timer_free(handshake_timer);
nbd_client_receive_next_request(client);
}
diff --git a/nbd/trace-events b/nbd/trace-events
index c4919a2dd5..553546f1f2 100644
--- a/nbd/trace-events
+++ b/nbd/trace-events
@@ -73,3 +73,4 @@ nbd_co_receive_request_decode_type(uint64_t handle, uint16_t type, const char *n
nbd_co_receive_request_payload_received(uint64_t handle, uint32_t len) "Payload received: handle = %" PRIu64 ", len = %" PRIu32
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
nbd_trip(void) "Reading request"
+nbd_handshake_timer_cb(void) "client took too long to negotiate"
--
2.39.3

View File

@ -0,0 +1,174 @@
From 0d204cb81aec2b13254a0bd53938f53bfea81cb5 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Wed, 7 Aug 2024 08:50:01 -0500
Subject: [PATCH 1/5] nbd/server: Plumb in new args to nbd_client_add()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
RH-Author: Eric Blake <eblake@redhat.com>
RH-MergeRequest: 388: nbd/server: fix CVE-2024-7409 (qemu crash on nbd-server-stop) [rhel-8.10.z]
RH-Jira: RHEL-52611
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
RH-Acked-by: Richard W.M. Jones <rjones@redhat.com>
RH-Commit: [1/4] 292be8dd2df2a840b2200e31a27e9d17fdab91ad (ebblake/qemu-kvm)
Upcoming patches to fix a CVE need to track an opaque pointer passed
in by the owner of a client object, as well as request for a time
limit on how fast negotiation must complete. Prepare for that by
changing the signature of nbd_client_new() and adding an accessor to
get at the opaque pointer, although for now the two servers
(qemu-nbd.c and blockdev-nbd.c) do not change behavior even though
they pass in a new default timeout value.
Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20240807174943.771624-11-eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
[eblake: s/LIMIT/MAX_SECS/ as suggested by Dan]
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit fb1c2aaa981e0a2fa6362c9985f1296b74f055ac)
Jira: https://issues.redhat.com/browse/RHEL-52611
Signed-off-by: Eric Blake <eblake@redhat.com>
---
blockdev-nbd.c | 6 ++++--
include/block/nbd.h | 11 ++++++++++-
nbd/server.c | 20 +++++++++++++++++---
qemu-nbd.c | 4 +++-
4 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index bdfa7ed3a5..b9e8dc78f3 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -59,8 +59,10 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
nbd_update_server_watch(nbd_server);
qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server");
- nbd_client_new(cioc, nbd_server->tlscreds, nbd_server->tlsauthz,
- nbd_blockdev_client_closed);
+ /* TODO - expose handshake timeout as QMP option */
+ nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_MAX_SECS,
+ nbd_server->tlscreds, nbd_server->tlsauthz,
+ nbd_blockdev_client_closed, NULL);
}
static void nbd_update_server_watch(NBDServerData *s)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 78d101b774..b71a297249 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -27,6 +27,12 @@
extern const BlockExportDriver blk_exp_nbd;
+/*
+ * NBD_DEFAULT_HANDSHAKE_MAX_SECS: Number of seconds in which client must
+ * succeed at NBD_OPT_GO before being forcefully dropped as too slow.
+ */
+#define NBD_DEFAULT_HANDSHAKE_MAX_SECS 10
+
/* Handshake phase structs - this struct is passed on the wire */
struct NBDOption {
@@ -338,9 +344,12 @@ AioContext *nbd_export_aio_context(NBDExport *exp);
NBDExport *nbd_export_find(const char *name);
void nbd_client_new(QIOChannelSocket *sioc,
+ uint32_t handshake_max_secs,
QCryptoTLSCreds *tlscreds,
const char *tlsauthz,
- void (*close_fn)(NBDClient *, bool));
+ void (*close_fn)(NBDClient *, bool),
+ void *owner);
+void *nbd_client_owner(NBDClient *client);
void nbd_client_get(NBDClient *client);
void nbd_client_put(NBDClient *client);
diff --git a/nbd/server.c b/nbd/server.c
index 6db124cf53..cc1b6838bf 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -120,10 +120,12 @@ typedef struct NBDExportMetaContexts {
struct NBDClient {
int refcount;
void (*close_fn)(NBDClient *client, bool negotiated);
+ void *owner;
NBDExport *exp;
QCryptoTLSCreds *tlscreds;
char *tlsauthz;
+ uint32_t handshake_max_secs;
QIOChannelSocket *sioc; /* The underlying data channel */
QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */
@@ -2706,6 +2708,7 @@ static coroutine_fn void nbd_co_client_start(void *opaque)
qemu_co_mutex_init(&client->send_lock);
+ /* TODO - utilize client->handshake_max_secs */
if (nbd_negotiate(client, &local_err)) {
if (local_err) {
error_report_err(local_err);
@@ -2718,14 +2721,17 @@ static coroutine_fn void nbd_co_client_start(void *opaque)
}
/*
- * Create a new client listener using the given channel @sioc.
+ * Create a new client listener using the given channel @sioc and @owner.
* Begin servicing it in a coroutine. When the connection closes, call
- * @close_fn with an indication of whether the client completed negotiation.
+ * @close_fn with an indication of whether the client completed negotiation
+ * within @handshake_max_secs seconds (0 for unbounded).
*/
void nbd_client_new(QIOChannelSocket *sioc,
+ uint32_t handshake_max_secs,
QCryptoTLSCreds *tlscreds,
const char *tlsauthz,
- void (*close_fn)(NBDClient *, bool))
+ void (*close_fn)(NBDClient *, bool),
+ void *owner)
{
NBDClient *client;
Coroutine *co;
@@ -2737,13 +2743,21 @@ void nbd_client_new(QIOChannelSocket *sioc,
object_ref(OBJECT(client->tlscreds));
}
client->tlsauthz = g_strdup(tlsauthz);
+ client->handshake_max_secs = handshake_max_secs;
client->sioc = sioc;
qio_channel_set_delay(QIO_CHANNEL(sioc), false);
object_ref(OBJECT(client->sioc));
client->ioc = QIO_CHANNEL(sioc);
object_ref(OBJECT(client->ioc));
client->close_fn = close_fn;
+ client->owner = owner;
co = qemu_coroutine_create(nbd_co_client_start, client);
qemu_coroutine_enter(co);
}
+
+void *
+nbd_client_owner(NBDClient *client)
+{
+ return client->owner;
+}
diff --git a/qemu-nbd.c b/qemu-nbd.c
index c6c20df68a..f48abf379e 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -363,7 +363,9 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
nb_fds++;
nbd_update_server_watch();
- nbd_client_new(cioc, tlscreds, tlsauthz, nbd_client_closed);
+ /* TODO - expose handshake timeout as command line option */
+ nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_MAX_SECS,
+ tlscreds, tlsauthz, nbd_client_closed, NULL);
}
static void nbd_update_server_watch(void)
--
2.39.3

View File

@ -0,0 +1,92 @@
From 7ad4fc282b1f96d619ce2f9f7ed9049c3b894dd4 Mon Sep 17 00:00:00 2001
From: Jon Maloy <jmaloy@redhat.com>
Date: Thu, 18 Jul 2024 09:42:42 -0400
Subject: [PATCH 1/6] virtio-gpu: free BHs, by implementing unrealize
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
RH-Author: Jon Maloy <jmaloy@redhat.com>
RH-MergeRequest: 380: QEMU: virtio: DMA reentrancy issue leads to double free vulnerability
RH-Jira: RHEL-32276
RH-Acked-by: Gerd Hoffmann <None>
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
RH-Commit: [1/6] d05c10426afac428d775669748f0aa689c23e787 (redhat/rhel/src/qemu-kvm/jons-qemu-kvm-2)
JIRA: https://issues.redhat.com/browse/RHEL-32276
CVE: CVE-2024-3446
Upstream: Merged
commit 957d77863e4564454eb97f8f371096843daf4678
Author: Marc-André Lureau <marcandre.lureau@redhat.com>
Date: Wed Jul 26 21:39:28 2023 +0400
virtio-gpu: free BHs, by implementing unrealize
Acked-by: Dongwon Kim <dongwon.kim@intel.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20230726173929.690601-2-marcandre.lureau@redhat.com>
Signed-off-by: Jon Maloy <jmaloy@redhat.com>
---
hw/display/virtio-gpu-base.c | 2 +-
hw/display/virtio-gpu.c | 10 ++++++++++
include/hw/virtio/virtio-gpu.h | 1 +
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/hw/display/virtio-gpu-base.c b/hw/display/virtio-gpu-base.c
index c8da4806e0..e3ff9dcf38 100644
--- a/hw/display/virtio-gpu-base.c
+++ b/hw/display/virtio-gpu-base.c
@@ -223,7 +223,7 @@ virtio_gpu_base_set_features(VirtIODevice *vdev, uint64_t features)
trace_virtio_gpu_features(((features & virgl) == virgl));
}
-static void
+void
virtio_gpu_base_device_unrealize(DeviceState *qdev)
{
VirtIOGPUBase *g = VIRTIO_GPU_BASE(qdev);
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index ecf9079145..e230e5091f 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1341,6 +1341,15 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
QTAILQ_INIT(&g->fenceq);
}
+static void virtio_gpu_device_unrealize(DeviceState *qdev)
+{
+ VirtIOGPU *g = VIRTIO_GPU(qdev);
+
+ g_clear_pointer(&g->ctrl_bh, qemu_bh_delete);
+ g_clear_pointer(&g->cursor_bh, qemu_bh_delete);
+ virtio_gpu_base_device_unrealize(qdev);
+}
+
void virtio_gpu_reset(VirtIODevice *vdev)
{
VirtIOGPU *g = VIRTIO_GPU(vdev);
@@ -1436,6 +1445,7 @@ static void virtio_gpu_class_init(ObjectClass *klass, void *data)
vgbc->gl_flushed = virtio_gpu_handle_gl_flushed;
vdc->realize = virtio_gpu_device_realize;
+ vdc->unrealize = virtio_gpu_device_unrealize;
vdc->reset = virtio_gpu_reset;
vdc->get_config = virtio_gpu_get_config;
vdc->set_config = virtio_gpu_set_config;
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index acfba7c76c..4367d005f1 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -235,6 +235,7 @@ bool virtio_gpu_base_device_realize(DeviceState *qdev,
VirtIOHandleOutput ctrl_cb,
VirtIOHandleOutput cursor_cb,
Error **errp);
+void virtio_gpu_base_device_unrealize(DeviceState *qdev);
void virtio_gpu_base_reset(VirtIOGPUBase *g);
void virtio_gpu_base_fill_display_info(VirtIOGPUBase *g,
struct virtio_gpu_resp_display_info *dpy_info);
--
2.39.3

View File

@ -0,0 +1,143 @@
From 29328e9693aeae1c980a859d4966deda9f54242d Mon Sep 17 00:00:00 2001
From: Jon Maloy <jmaloy@redhat.com>
Date: Thu, 18 Jul 2024 09:36:06 -0400
Subject: [PATCH 2/6] virtio-gpu: reset gfx resources in main thread
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
RH-Author: Jon Maloy <jmaloy@redhat.com>
RH-MergeRequest: 380: QEMU: virtio: DMA reentrancy issue leads to double free vulnerability
RH-Jira: RHEL-32276
RH-Acked-by: Gerd Hoffmann <None>
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
RH-Commit: [2/6] a97eef1e6e85b44c08d17adcdc468e857e48a17e (redhat/rhel/src/qemu-kvm/jons-qemu-kvm-2)
JIRA: https://issues.redhat.com/browse/RHEL-32276
CVE: CVE-2024-3446
Upstream: Merged
commit a41e2d97f92b48552988b3cc62dce79d62f60dcc
Author: Marc-André Lureau <marcandre.lureau@redhat.com>
Date: Wed Jul 26 21:39:29 2023 +0400
virtio-gpu: reset gfx resources in main thread
Calling OpenGL from different threads can have bad consequences if not
carefully reviewed. It's not generally supported. In my case, I was
debugging a crash in glDeleteTextures from OPENGL32.DLL, where I asked
qemu for gl=es, and thus ANGLE implementation was expected. libepoxy did
resolution of the global pointer for glGenTexture to the GLES version
from the main thread. But it resolved glDeleteTextures to the GL
version, because it was done from a different thread without correct
context. Oops.
Let's stick to the main thread for GL calls by using a BH.
Note: I didn't use atomics for reset_finished check, assuming the BQL
will provide enough of sync, but I might be wrong.
Acked-by: Dongwon Kim <dongwon.kim@intel.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20230726173929.690601-3-marcandre.lureau@redhat.com>
Signed-off-by: Jon Maloy <jmaloy@redhat.com>
---
hw/display/virtio-gpu.c | 35 +++++++++++++++++++++++++++++++---
include/hw/virtio/virtio-gpu.h | 3 +++
2 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index e230e5091f..c28ce1ea72 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -14,6 +14,7 @@
#include "qemu/osdep.h"
#include "qemu/units.h"
#include "qemu/iov.h"
+#include "sysemu/cpus.h"
#include "ui/console.h"
#include "trace.h"
#include "sysemu/dma.h"
@@ -42,6 +43,7 @@ virtio_gpu_find_check_resource(VirtIOGPU *g, uint32_t resource_id,
static void virtio_gpu_cleanup_mapping(VirtIOGPU *g,
struct virtio_gpu_simple_resource *res);
+static void virtio_gpu_reset_bh(void *opaque);
void virtio_gpu_update_cursor_data(VirtIOGPU *g,
struct virtio_gpu_scanout *s,
@@ -1336,6 +1338,8 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
&qdev->mem_reentrancy_guard);
g->cursor_bh = qemu_bh_new_guarded(virtio_gpu_cursor_bh, g,
&qdev->mem_reentrancy_guard);
+ g->reset_bh = qemu_bh_new(virtio_gpu_reset_bh, g);
+ qemu_cond_init(&g->reset_cond);
QTAILQ_INIT(&g->reslist);
QTAILQ_INIT(&g->cmdq);
QTAILQ_INIT(&g->fenceq);
@@ -1347,19 +1351,44 @@ static void virtio_gpu_device_unrealize(DeviceState *qdev)
g_clear_pointer(&g->ctrl_bh, qemu_bh_delete);
g_clear_pointer(&g->cursor_bh, qemu_bh_delete);
+ g_clear_pointer(&g->reset_bh, qemu_bh_delete);
+ qemu_cond_destroy(&g->reset_cond);
virtio_gpu_base_device_unrealize(qdev);
}
-void virtio_gpu_reset(VirtIODevice *vdev)
+static void virtio_gpu_reset_bh(void *opaque)
{
- VirtIOGPU *g = VIRTIO_GPU(vdev);
+ VirtIOGPU *g = VIRTIO_GPU(opaque);
struct virtio_gpu_simple_resource *res, *tmp;
- struct virtio_gpu_ctrl_command *cmd;
+ int i = 0;
QTAILQ_FOREACH_SAFE(res, &g->reslist, next, tmp) {
virtio_gpu_resource_destroy(g, res);
}
+ for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
+ dpy_gfx_replace_surface(g->parent_obj.scanout[i].con, NULL);
+ }
+
+ g->reset_finished = true;
+ qemu_cond_signal(&g->reset_cond);
+}
+
+void virtio_gpu_reset(VirtIODevice *vdev)
+{
+ VirtIOGPU *g = VIRTIO_GPU(vdev);
+ struct virtio_gpu_ctrl_command *cmd;
+
+ if (qemu_in_vcpu_thread()) {
+ g->reset_finished = false;
+ qemu_bh_schedule(g->reset_bh);
+ while (!g->reset_finished) {
+ qemu_cond_wait_iothread(&g->reset_cond);
+ }
+ } else {
+ virtio_gpu_reset_bh(g);
+ }
+
while (!QTAILQ_EMPTY(&g->cmdq)) {
cmd = QTAILQ_FIRST(&g->cmdq);
QTAILQ_REMOVE(&g->cmdq, cmd, next);
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 4367d005f1..f3578c1325 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -166,6 +166,9 @@ struct VirtIOGPU {
QEMUBH *ctrl_bh;
QEMUBH *cursor_bh;
+ QEMUBH *reset_bh;
+ QemuCond reset_cond;
+ bool reset_finished;
QTAILQ_HEAD(, virtio_gpu_simple_resource) reslist;
QTAILQ_HEAD(, virtio_gpu_ctrl_command) cmdq;
--
2.39.3

View File

@ -0,0 +1,49 @@
From a38e51982522910475ec051f81116639254a2955 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Thu, 30 May 2024 13:10:29 +0200
Subject: [PATCH 5/5] vnc: increase max display size
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
RH-Author: Marc-André Lureau <marcandre.lureau@redhat.com>
RH-MergeRequest: 391: vnc: increase max display size
RH-Jira: RHEL-50854
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
RH-Commit: [1/1] 8d79bbc6949ca7264f6701121b47e946eb8ac824
Resolves:
https://issues.redhat.com/browse/RHEL-50854
It's 2024. 4k display resolutions are a thing these days.
Raise width and height limits of the qemu vnc server.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1596
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-ID: <20240530111029.1726329-1-kraxel@redhat.com>
(cherry picked from commit 1f1736a8f16d27a99abd371caaeedc10e6411d15)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
ui/vnc.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ui/vnc.h b/ui/vnc.h
index a7149831f9..4d44957cc2 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -81,8 +81,8 @@ typedef void VncSendHextileTile(VncState *vs,
/* VNC_MAX_WIDTH must be a multiple of VNC_DIRTY_PIXELS_PER_BIT. */
-#define VNC_MAX_WIDTH ROUND_UP(2560, VNC_DIRTY_PIXELS_PER_BIT)
-#define VNC_MAX_HEIGHT 2048
+#define VNC_MAX_WIDTH ROUND_UP(5120, VNC_DIRTY_PIXELS_PER_BIT)
+#define VNC_MAX_HEIGHT 2160
/* VNC_DIRTY_BITS is the number of bits in the dirty bitmap. */
#define VNC_DIRTY_BITS (VNC_MAX_WIDTH / VNC_DIRTY_PIXELS_PER_BIT)
--
2.39.3

View File

@ -83,7 +83,7 @@ Obsoletes: %1-rhev <= %{epoch}:%{version}-%{release}
Summary: QEMU is a machine emulator and virtualizer Summary: QEMU is a machine emulator and virtualizer
Name: qemu-kvm Name: qemu-kvm
Version: 6.2.0 Version: 6.2.0
Release: 50%{?rcrel}%{?dist} Release: 52%{?rcrel}%{?dist}
# Epoch because we pushed a qemu-1.0 package. AIUI this can't ever be dropped # Epoch because we pushed a qemu-1.0 package. AIUI this can't ever be dropped
Epoch: 15 Epoch: 15
License: GPLv2 and GPLv2+ and CC-BY License: GPLv2 and GPLv2+ and CC-BY
@ -857,6 +857,28 @@ Patch347: kvm-iotests-270-Don-t-store-data-file-with-json-prefix-i.patch
Patch348: kvm-block-introduce-bdrv_open_file_child-helper.patch Patch348: kvm-block-introduce-bdrv_open_file_child-helper.patch
# For RHEL-35616 - CVE-2024-4467 virt:rhel/qemu-kvm: QEMU: 'qemu-img info' leads to host file read/write [rhel-8.10.z] # For RHEL-35616 - CVE-2024-4467 virt:rhel/qemu-kvm: QEMU: 'qemu-img info' leads to host file read/write [rhel-8.10.z]
Patch349: kvm-block-Parse-filenames-only-when-explicitly-requested.patch Patch349: kvm-block-Parse-filenames-only-when-explicitly-requested.patch
# For RHEL-32276 - CVE-2024-3446 virt:rhel/qemu-kvm: QEMU: virtio: DMA reentrancy issue leads to double free vulnerability [rhel-8]
Patch350: kvm-virtio-gpu-free-BHs-by-implementing-unrealize.patch
# For RHEL-32276 - CVE-2024-3446 virt:rhel/qemu-kvm: QEMU: virtio: DMA reentrancy issue leads to double free vulnerability [rhel-8]
Patch351: kvm-virtio-gpu-reset-gfx-resources-in-main-thread.patch
# For RHEL-32276 - CVE-2024-3446 virt:rhel/qemu-kvm: QEMU: virtio: DMA reentrancy issue leads to double free vulnerability [rhel-8]
Patch352: kvm-hw-virtio-Introduce-virtio_bh_new_guarded-helper.patch
# For RHEL-32276 - CVE-2024-3446 virt:rhel/qemu-kvm: QEMU: virtio: DMA reentrancy issue leads to double free vulnerability [rhel-8]
Patch353: kvm-hw-display-virtio-gpu-Protect-from-DMA-re-entrancy-b.patch
# For RHEL-32276 - CVE-2024-3446 virt:rhel/qemu-kvm: QEMU: virtio: DMA reentrancy issue leads to double free vulnerability [rhel-8]
Patch354: kvm-hw-char-virtio-serial-bus-Protect-from-DMA-re-entran.patch
# For RHEL-32276 - CVE-2024-3446 virt:rhel/qemu-kvm: QEMU: virtio: DMA reentrancy issue leads to double free vulnerability [rhel-8]
Patch355: kvm-hw-virtio-virtio-crypto-Protect-from-DMA-re-entrancy.patch
# For RHEL-52611 - CVE-2024-7409 virt:rhel/qemu-kvm: Denial of Service via Improper Synchronization in QEMU NBD Server During Socket Closure [rhel-8.10.z]
Patch356: kvm-nbd-server-Plumb-in-new-args-to-nbd_client_add.patch
# For RHEL-52611 - CVE-2024-7409 virt:rhel/qemu-kvm: Denial of Service via Improper Synchronization in QEMU NBD Server During Socket Closure [rhel-8.10.z]
Patch357: kvm-nbd-server-CVE-2024-7409-Cap-default-max-connections.patch
# For RHEL-52611 - CVE-2024-7409 virt:rhel/qemu-kvm: Denial of Service via Improper Synchronization in QEMU NBD Server During Socket Closure [rhel-8.10.z]
Patch358: kvm-nbd-server-CVE-2024-7409-Drop-non-negotiating-client.patch
# For RHEL-52611 - CVE-2024-7409 virt:rhel/qemu-kvm: Denial of Service via Improper Synchronization in QEMU NBD Server During Socket Closure [rhel-8.10.z]
Patch359: kvm-nbd-server-CVE-2024-7409-Close-stray-clients-at-serv.patch
# For RHEL-50854 - vnc: increase max display size to 4K
Patch360: kvm-vnc-increase-max-display-size.patch
BuildRequires: wget BuildRequires: wget
BuildRequires: rpm-build BuildRequires: rpm-build
@ -2026,6 +2048,27 @@ sh %{_sysconfdir}/sysconfig/modules/kvm.modules &> /dev/null || :
%changelog %changelog
* Wed Aug 21 2024 Miroslav Rezanina <mrezanin@redhat.com> - 6.2.0-52.el8
- kvm-nbd-server-Plumb-in-new-args-to-nbd_client_add.patch [RHEL-52611]
- kvm-nbd-server-CVE-2024-7409-Cap-default-max-connections.patch [RHEL-52611]
- kvm-nbd-server-CVE-2024-7409-Drop-non-negotiating-client.patch [RHEL-52611]
- kvm-nbd-server-CVE-2024-7409-Close-stray-clients-at-serv.patch [RHEL-52611]
- kvm-vnc-increase-max-display-size.patch [RHEL-50854]
- Resolves: RHEL-52611
(CVE-2024-7409 virt:rhel/qemu-kvm: Denial of Service via Improper Synchronization in QEMU NBD Server During Socket Closure [rhel-8.10.z])
- Resolves: RHEL-50854
(vnc: increase max display size to 4K)
* Mon Jul 29 2024 Miroslav Rezanina <mrezanin@redhat.com> - 6.2.0-51.el8
- kvm-virtio-gpu-free-BHs-by-implementing-unrealize.patch [RHEL-32276]
- kvm-virtio-gpu-reset-gfx-resources-in-main-thread.patch [RHEL-32276]
- kvm-hw-virtio-Introduce-virtio_bh_new_guarded-helper.patch [RHEL-32276]
- kvm-hw-display-virtio-gpu-Protect-from-DMA-re-entrancy-b.patch [RHEL-32276]
- kvm-hw-char-virtio-serial-bus-Protect-from-DMA-re-entran.patch [RHEL-32276]
- kvm-hw-virtio-virtio-crypto-Protect-from-DMA-re-entrancy.patch [RHEL-32276]
- Resolves: RHEL-32276
(CVE-2024-3446 virt:rhel/qemu-kvm: QEMU: virtio: DMA reentrancy issue leads to double free vulnerability [rhel-8])
* Thu Jul 04 2024 Miroslav Rezanina <mrezanin@redhat.com> - 6.2.0-50 * Thu Jul 04 2024 Miroslav Rezanina <mrezanin@redhat.com> - 6.2.0-50
- kvm-qcow2-Don-t-open-data_file-with-BDRV_O_NO_IO.patch [RHEL-35616] - kvm-qcow2-Don-t-open-data_file-with-BDRV_O_NO_IO.patch [RHEL-35616]
- kvm-iotests-244-Don-t-store-data-file-with-protocol-in-i.patch [RHEL-35616] - kvm-iotests-244-Don-t-store-data-file-with-protocol-in-i.patch [RHEL-35616]