CVE-2015-7295: virtio-net possible remote DoS (bz #1264393)

drive-mirror: Fix coroutine reentrance (bz #1266936)
Fix udp socket 'localaddr' (bz #1268708)
This commit is contained in:
Cole Robinson 2015-10-08 13:39:40 -04:00
parent 94d6f121d6
commit aeebdca142
6 changed files with 330 additions and 1 deletions

View File

@ -0,0 +1,54 @@
From: Jason Wang <jasowang@redhat.com>
Date: Fri, 25 Sep 2015 13:21:28 +0800
Subject: [PATCH] virtio: introduce virtqueue_unmap_sg()
Factor out sg unmapping logic. This will be reused by the patch that
can discard descriptor.
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Andrew James <andrew.james@hpe.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
(cherry picked from commit ce317461573bac12b10d67699b4ddf1f97cf066c)
---
hw/virtio/virtio.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 17c1260..7d88f26 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -240,14 +240,12 @@ int virtio_queue_empty(VirtQueue *vq)
return vring_avail_idx(vq) == vq->last_avail_idx;
}
-void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
- unsigned int len, unsigned int idx)
+static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem,
+ unsigned int len)
{
unsigned int offset;
int i;
- trace_virtqueue_fill(vq, elem, len, idx);
-
offset = 0;
for (i = 0; i < elem->in_num; i++) {
size_t size = MIN(len - offset, elem->in_sg[i].iov_len);
@@ -263,6 +261,14 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
cpu_physical_memory_unmap(elem->out_sg[i].iov_base,
elem->out_sg[i].iov_len,
0, elem->out_sg[i].iov_len);
+}
+
+void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
+ unsigned int len, unsigned int idx)
+{
+ trace_virtqueue_fill(vq, elem, len, idx);
+
+ virtqueue_unmap_sg(vq, elem, len);
idx = (idx + vring_used_idx(vq)) % vq->vring.num;

View File

@ -0,0 +1,50 @@
From: Jason Wang <jasowang@redhat.com>
Date: Fri, 25 Sep 2015 13:21:29 +0800
Subject: [PATCH] virtio: introduce virtqueue_discard()
This patch introduces virtqueue_discard() to discard a descriptor and
unmap the sgs. This will be used by the patch that will discard
descriptor when packet is truncated.
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
(cherry picked from commit 29b9f5efd78ae0f9cc02dd169b6e80d2c404bade)
---
hw/virtio/virtio.c | 7 +++++++
include/hw/virtio/virtio.h | 2 ++
2 files changed, 9 insertions(+)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 7d88f26..e9f72a5 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -263,6 +263,13 @@ static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem,
0, elem->out_sg[i].iov_len);
}
+void virtqueue_discard(VirtQueue *vq, const VirtQueueElement *elem,
+ unsigned int len)
+{
+ vq->last_avail_idx--;
+ virtqueue_unmap_sg(vq, elem, len);
+}
+
void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
unsigned int len, unsigned int idx)
{
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index d95f8b6..1899654 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -136,6 +136,8 @@ void virtio_del_queue(VirtIODevice *vdev, int n);
void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem,
unsigned int len);
void virtqueue_flush(VirtQueue *vq, unsigned int count);
+void virtqueue_discard(VirtQueue *vq, const VirtQueueElement *elem,
+ unsigned int len);
void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
unsigned int len, unsigned int idx);

View File

@ -0,0 +1,43 @@
From: Jason Wang <jasowang@redhat.com>
Date: Fri, 25 Sep 2015 13:21:30 +0800
Subject: [PATCH] virtio-net: correctly drop truncated packets
When packet is truncated during receiving, we drop the packets but
neither discard the descriptor nor add and signal used
descriptor. This will lead several issues:
- sg mappings are leaked
- rx will be stalled if a lots of packets were truncated
In order to be consistent with vhost, fix by discarding the descriptor
in this case.
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
(cherry picked from commit 0cf33fb6b49a19de32859e2cdc6021334f448fb3)
---
hw/net/virtio-net.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 2d570e4..48b6655 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1073,13 +1073,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t
* must have consumed the complete packet.
* Otherwise, drop it. */
if (!n->mergeable_rx_bufs && offset < size) {
-#if 0
- error_report("virtio-net truncated non-mergeable packet: "
- "i %zd mergeable %d offset %zd, size %zd, "
- "guest hdr len %zd, host hdr len %zd",
- i, n->mergeable_rx_bufs,
- offset, size, n->guest_hdr_len, n->host_hdr_len);
-#endif
+ virtqueue_discard(q->rx_vq, &elem, total);
return size;
}

View File

@ -0,0 +1,117 @@
From: Kevin Wolf <kwolf@redhat.com>
Date: Thu, 13 Aug 2015 10:41:50 +0200
Subject: [PATCH] mirror: Fix coroutine reentrance
This fixes a regression introduced by commit dcfb3beb ("mirror: Do zero
write on target if sectors not allocated"), which was reported to cause
aborts with the message "Co-routine re-entered recursively".
The cause for this bug is the following code in mirror_iteration_done():
if (s->common.busy) {
qemu_coroutine_enter(s->common.co, NULL);
}
This has always been ugly because - unlike most places that reenter - it
doesn't have a specific yield that it pairs with, but is more
uncontrolled. What we really mean here is "reenter the coroutine if
it's in one of the four explicit yields in mirror.c".
This used to be equivalent with s->common.busy because neither
mirror_run() nor mirror_iteration() call any function that could yield.
However since commit dcfb3beb this doesn't hold true any more:
bdrv_get_block_status_above() can yield.
So what happens is that bdrv_get_block_status_above() wants to take a
lock that is already held, so it adds itself to the queue of waiting
coroutines and yields. Instead of being woken up by the unlock function,
however, it gets woken up by mirror_iteration_done(), which is obviously
wrong.
In most cases the code actually happens to cope fairly well with such
cases, but in this specific case, the unlock must already have scheduled
the coroutine for wakeup when mirror_iteration_done() reentered it. And
then the coroutine happened to process the scheduled restarts and tried
to reenter itself recursively.
This patch fixes the problem by pairing the reenter in
mirror_iteration_done() with specific yields instead of abusing
s->common.busy.
Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Message-id: 1439455310-11263-1-git-send-email-kwolf@redhat.com
Signed-off-by: Jeff Cody <jcody@redhat.com>
(cherry picked from commit e424aff5f307227b1c2512bbb8ece891bb895cef)
---
block/mirror.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index 6f1bc3c..d06a0be 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -58,6 +58,7 @@ typedef struct MirrorBlockJob {
int sectors_in_flight;
int ret;
bool unmap;
+ bool waiting_for_io;
} MirrorBlockJob;
typedef struct MirrorOp {
@@ -112,11 +113,7 @@ static void mirror_iteration_done(MirrorOp *op, int ret)
qemu_iovec_destroy(&op->qiov);
g_slice_free(MirrorOp, op);
- /* Enter coroutine when it is not sleeping. The coroutine sleeps to
- * rate-limit itself. The coroutine will eventually resume since there is
- * a sleep timeout so don't wake it early.
- */
- if (s->common.busy) {
+ if (s->waiting_for_io) {
qemu_coroutine_enter(s->common.co, NULL);
}
}
@@ -206,7 +203,9 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
/* Wait for I/O to this cluster (from a previous iteration) to be done. */
while (test_bit(next_chunk, s->in_flight_bitmap)) {
trace_mirror_yield_in_flight(s, sector_num, s->in_flight);
+ s->waiting_for_io = true;
qemu_coroutine_yield();
+ s->waiting_for_io = false;
}
do {
@@ -242,7 +241,9 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
*/
while (nb_chunks == 0 && s->buf_free_count < added_chunks) {
trace_mirror_yield_buf_busy(s, nb_chunks, s->in_flight);
+ s->waiting_for_io = true;
qemu_coroutine_yield();
+ s->waiting_for_io = false;
}
if (s->buf_free_count < nb_chunks + added_chunks) {
trace_mirror_break_buf_busy(s, nb_chunks, s->in_flight);
@@ -341,7 +342,9 @@ static void mirror_free_init(MirrorBlockJob *s)
static void mirror_drain(MirrorBlockJob *s)
{
while (s->in_flight > 0) {
+ s->waiting_for_io = true;
qemu_coroutine_yield();
+ s->waiting_for_io = false;
}
}
@@ -516,7 +519,9 @@ static void coroutine_fn mirror_run(void *opaque)
if (s->in_flight == MAX_IN_FLIGHT || s->buf_free_count == 0 ||
(cnt == 0 && s->in_flight > 0)) {
trace_mirror_yield(s, s->in_flight, s->buf_free_count, cnt);
+ s->waiting_for_io = true;
qemu_coroutine_yield();
+ s->waiting_for_io = false;
continue;
} else if (cnt != 0) {
delay_ns = mirror_iteration(s);

View File

@ -0,0 +1,52 @@
From: Peter Krempa <pkrempa@redhat.com>
Date: Fri, 15 May 2015 11:31:43 +0200
Subject: [PATCH] util: socket: Add missing localaddr and localport option for
DGRAM socket
The 'socket_optslist' structure does not contain the 'localaddr' and
'localport' options that are parsed in case you are creating a
'connect' type UDP character device.
I've noticed it happening after commit f43e47dbf6de24db20ec9b588bb6cc762
made qemu abort() after seeing the invalid option.
A minimal reproducer for the case is:
$ qemu-system-x86_64 -chardev udp,id=charrng0,host=127.0.0.1,port=1234,localaddr=,localport=1234
qemu-system-x86_64: -chardev udp,id=charrng0,host=127.0.0.1,port=1234,localaddr=,localport=1234: Invalid parameter 'localaddr'
Aborted (core dumped)
Prior to the commit mentioned above the error would be printed but the
value for localaddr and localport was simply ignored. I did not go
through the code to find out when it was broken.
Add the two fields so that the options can again be parsed correctly and
qemu doesn't abort().
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1220252
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
(cherry picked from commit b8981dc9aae25fa79e5f35609e63f50f078a572d)
---
util/qemu-sockets.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 87c9bc6..72066be 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -45,6 +45,12 @@ QemuOptsList socket_optslist = {
.name = "port",
.type = QEMU_OPT_STRING,
},{
+ .name = "localaddr",
+ .type = QEMU_OPT_STRING,
+ },{
+ .name = "localport",
+ .type = QEMU_OPT_STRING,
+ },{
.name = "to",
.type = QEMU_OPT_NUMBER,
},{

View File

@ -43,7 +43,7 @@
Summary: QEMU is a FAST! processor emulator
Name: qemu
Version: 2.3.1
Release: 6%{?dist}
Release: 7%{?dist}
Epoch: 2
License: GPLv2+ and LGPLv2+ and BSD
Group: Development/Tools
@ -106,6 +106,14 @@ Patch0013: 0013-block-mirror-Sleep-periodically-during-bitmap-scanni.patch
# guests
Patch0014: 0014-target-ppc-fix-vcipher-vcipherlast-vncipherlast-and-.patch
Patch0015: 0015-target-ppc-fix-xscmpodp-and-xscmpudp-decoding.patch
# CVE-2015-7295: virtio-net possible remote DoS (bz #1264393)
Patch0016: 0016-virtio-introduce-virtqueue_unmap_sg.patch
Patch0017: 0017-virtio-introduce-virtqueue_discard.patch
Patch0018: 0018-virtio-net-correctly-drop-truncated-packets.patch
# drive-mirror: Fix coroutine reentrance (bz #1266936)
Patch0019: 0019-mirror-Fix-coroutine-reentrance.patch
# Fix udp socket 'localaddr' (bz #1268708)
Patch0020: 0020-util-socket-Add-missing-localaddr-and-localport-opti.patch
BuildRequires: SDL2-devel
BuildRequires: zlib-devel
@ -1208,6 +1216,11 @@ getent passwd qemu >/dev/null || \
%changelog
* Thu Oct 08 2015 Cole Robinson <crobinso@redhat.com> - 2:2.3.1-7
- CVE-2015-7295: virtio-net possible remote DoS (bz #1264393)
- drive-mirror: Fix coroutine reentrance (bz #1266936)
- Fix udp socket 'localaddr' (bz #1268708)
* Tue Sep 22 2015 Cole Robinson <crobinso@redhat.com> - 2:2.3.1-6
- Fix emulation of various instructions, required by libm in F22 ppc64 guests
- Re-add patches accidentally dropped in last build