File 0012-Fix-race-condition-in-red_get_data_chunks_... of Package spice.1097

Overview Repositories Revisions Requests Users Attributes Meta

File 0012-Fix-race-condition-in-red_get_data_chunks_ptr.patch of Package spice.1097

From 3738478ed7065fe05f3ee4848f8a7fcdf40aa920 Mon Sep 17 00:00:00 2001
From: Frediano Ziglio <fziglio@redhat.com>
Date: Tue, 8 Sep 2015 12:12:19 +0100
Subject: [PATCH 12/19] Fix race condition in red_get_data_chunks_ptr

Do not read multiple times data from guest as this can be changed by
other guest vcpus. This causes races and security problems if these
data are used for buffer allocation or checks.

Actually, the 'data' member can't change during read as it is just a
pointer to a fixed array contained in qxl. However, this change will
make it clear that there can be no race condition.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
 server/red_parse_qxl.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
index cfa21f9..2863ae2 100644
--- a/server/red_parse_qxl.c
+++ b/server/red_parse_qxl.c
@@ -102,30 +102,33 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo *slots, int group_id,
     RedDataChunk *red_prev;
     size_t data_size = 0;
     int error;
+    QXLPHYSICAL next_chunk;
 
     red->data_size = qxl->data_size;
     data_size += red->data_size;
-    if (!validate_virt(slots, (intptr_t)qxl->data, memslot_id, red->data_size, group_id)) {
+    red->data = qxl->data;
+    if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id)) {
+        red->data = NULL;
         return 0;
     }
-    red->data = qxl->data;
     red->prev_chunk = NULL;
 
-    while (qxl->next_chunk) {
+    while ((next_chunk = qxl->next_chunk) != 0) {
         red_prev = red;
         red = spice_new(RedDataChunk, 1);
-        memslot_id = get_memslot_id(slots, qxl->next_chunk);
-        qxl = (QXLDataChunk *)get_virt(slots, qxl->next_chunk, sizeof(*qxl), group_id,
+        memslot_id = get_memslot_id(slots, next_chunk);
+        qxl = (QXLDataChunk *)get_virt(slots, next_chunk, sizeof(*qxl), group_id,
                                       &error);
         if (error) {
             return 0;
         }
         red->data_size = qxl->data_size;
         data_size += red->data_size;
-        if (!validate_virt(slots, (intptr_t)qxl->data, memslot_id, red->data_size, group_id)) {
+        red->data = qxl->data;
+        if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id)) {
+            red->data = NULL;
             return 0;
         }
-        red->data = qxl->data;
         red->prev_chunk = red_prev;
         red_prev->next_chunk = red;
     }
-- 
2.1.4

Places

File 0012-Fix-race-condition-in-red_get_data_chunks_ptr.patch of Package spice.1097

Places