|
 |
73b8f2 |
From 20979131dee0f81972ca159e5f754d42890e75e6 Mon Sep 17 00:00:00 2001
|
|
|
73b8f2 |
From: Frediano Ziglio <fziglio@redhat.com>
|
|
|
73b8f2 |
Date: Tue, 8 Sep 2015 12:12:19 +0100
|
|
|
73b8f2 |
Subject: [PATCH 50/57] Fix race condition in red_get_data_chunks_ptr
|
|
|
73b8f2 |
|
|
|
73b8f2 |
Do not read multiple times data from guest as this can be changed by
|
|
|
73b8f2 |
other guest vcpus. This causes races and security problems if these
|
|
|
73b8f2 |
data are used for buffer allocation or checks.
|
|
|
73b8f2 |
|
|
|
73b8f2 |
Actually, the 'data' member can't change during read as it is just a
|
|
|
73b8f2 |
pointer to a fixed array contained in qxl. However, this change will
|
|
|
73b8f2 |
make it clear that there can be no race condition.
|
|
|
73b8f2 |
|
|
|
73b8f2 |
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
|
|
|
73b8f2 |
---
|
|
|
73b8f2 |
server/red_parse_qxl.c | 17 ++++++++++-------
|
|
|
73b8f2 |
1 file changed, 10 insertions(+), 7 deletions(-)
|
|
|
73b8f2 |
|
|
|
73b8f2 |
diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
|
|
|
73b8f2 |
index c4b82be..7cc20e6 100644
|
|
|
73b8f2 |
--- a/server/red_parse_qxl.c
|
|
|
73b8f2 |
+++ b/server/red_parse_qxl.c
|
|
|
73b8f2 |
@@ -102,30 +102,33 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo *slots, int group_id,
|
|
|
73b8f2 |
RedDataChunk *red_prev;
|
|
|
73b8f2 |
size_t data_size = 0;
|
|
|
73b8f2 |
int error;
|
|
|
73b8f2 |
+ QXLPHYSICAL next_chunk;
|
|
|
73b8f2 |
|
|
|
73b8f2 |
red->data_size = qxl->data_size;
|
|
|
73b8f2 |
data_size += red->data_size;
|
|
|
73b8f2 |
- if (!validate_virt(slots, (intptr_t)qxl->data, memslot_id, red->data_size, group_id)) {
|
|
|
73b8f2 |
+ red->data = qxl->data;
|
|
|
73b8f2 |
+ if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id)) {
|
|
|
73b8f2 |
+ red->data = NULL;
|
|
|
73b8f2 |
return 0;
|
|
|
73b8f2 |
}
|
|
|
73b8f2 |
- red->data = qxl->data;
|
|
|
73b8f2 |
red->prev_chunk = NULL;
|
|
|
73b8f2 |
|
|
|
73b8f2 |
- while (qxl->next_chunk) {
|
|
|
73b8f2 |
+ while ((next_chunk = qxl->next_chunk) != 0) {
|
|
|
73b8f2 |
red_prev = red;
|
|
|
73b8f2 |
red = spice_new(RedDataChunk, 1);
|
|
|
73b8f2 |
- memslot_id = get_memslot_id(slots, qxl->next_chunk);
|
|
|
73b8f2 |
- qxl = (QXLDataChunk *)get_virt(slots, qxl->next_chunk, sizeof(*qxl), group_id,
|
|
|
73b8f2 |
+ memslot_id = get_memslot_id(slots, next_chunk);
|
|
|
73b8f2 |
+ qxl = (QXLDataChunk *)get_virt(slots, next_chunk, sizeof(*qxl), group_id,
|
|
|
73b8f2 |
&error);
|
|
|
73b8f2 |
if (error) {
|
|
|
73b8f2 |
return 0;
|
|
|
73b8f2 |
}
|
|
|
73b8f2 |
red->data_size = qxl->data_size;
|
|
|
73b8f2 |
data_size += red->data_size;
|
|
|
73b8f2 |
- if (!validate_virt(slots, (intptr_t)qxl->data, memslot_id, red->data_size, group_id)) {
|
|
|
73b8f2 |
+ red->data = qxl->data;
|
|
|
73b8f2 |
+ if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id)) {
|
|
|
73b8f2 |
+ red->data = NULL;
|
|
|
73b8f2 |
return 0;
|
|
|
73b8f2 |
}
|
|
|
73b8f2 |
- red->data = qxl->data;
|
|
|
73b8f2 |
red->prev_chunk = red_prev;
|
|
|
73b8f2 |
red_prev->next_chunk = red;
|
|
|
73b8f2 |
}
|
|
|
73b8f2 |
--
|
|
|
73b8f2 |
2.4.3
|
|
|
73b8f2 |
|