[Spice-devel] [PATCH spice-server v2] red-parse-qxl: Fix QUIC images from QXL

Frediano Ziglio fziglio at redhat.com
Tue Jul 23 20:21:14 UTC 2019


The decoding is wrong, the Red and QXL structures are different so
code is not doing what is expected. red-parse-qxl translate from QXL
to Red structures, red-record-qxl saves Red structure to file,
red-replay-qxl is supposed to read from file into QXL directly.

If a Quic image is stored inside QXL memory the layout of the QXLImage
in memory is:
- QXLImageDescriptor
- QXLQUICData
- QXLDataChunk
- first chunk data
and all remaining data in linked QXLDataChunk.
red_replay_image was reading the image as data was all contained in
QXLImage->quic.data however "data" shoule store the first QXLDataChunk
followed by QXLDataChunk data.
Use proper base_size calling red_replay_data_chunks in order to
initialise the image with the first data chunk correctly.

Not easy to reproduce, the only driver is XDDM which means Windows XP
or similars, I got no recording with such images.

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

Changes v1:
- realloc the buffer inside read_binary with the proper size.
  The proper size if not available in red_replay_image.

diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index 674feae2f..1f1bd8c1d 100644
--- a/server/red-replay-qxl.c
+++ b/server/red-replay-qxl.c
@@ -114,6 +114,9 @@ static inline void replay_free(SpiceReplay *replay, void *mem)
 
 static inline void *replay_realloc(SpiceReplay *replay, void *mem, size_t n_bytes)
 {
+    if (mem == NULL) {
+        return replay_malloc(replay, n_bytes);
+    }
     GList *elem = g_list_find(replay->allocated, mem);
     elem->data = g_realloc(mem, n_bytes);
     return elem->data;
@@ -231,9 +234,7 @@ static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *siz
         return REPLAY_ERROR;
     }
 
-    if (*buf == NULL) {
-        *buf = replay_malloc(replay, *size + base_size);
-    }
+    *buf = replay_realloc(replay, *buf, *size + base_size);
 #if 0
     {
         int num_read = fread(*buf + base_size, *size, 1, fd);
@@ -497,9 +498,9 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
         if (replay->error) {
             return NULL;
         }
-        qxl = replay_realloc(replay, qxl, sizeof(QXLImageDescriptor) + sizeof(QXLQUICData) +
-                             qxl->quic.data_size);
-        size = red_replay_data_chunks(replay, "quic.data", (uint8_t**)&qxl->quic.data, 0);
+        size = red_replay_data_chunks(replay, "quic.data", (uint8_t**)&qxl,
+                                      sizeof(QXLImageDescriptor) + sizeof(QXLQUICData) +
+                                      sizeof(QXLDataChunk));
         spice_assert(size == qxl->quic.data_size);
         break;
     default:
@@ -526,7 +527,9 @@ static void red_replay_image_free(SpiceReplay *replay, QXLPHYSICAL p, uint32_t f
     case SPICE_IMAGE_TYPE_SURFACE:
         break;
     case SPICE_IMAGE_TYPE_QUIC:
-        red_replay_data_chunks_free(replay, qxl, 0);
+        red_replay_data_chunks_free(replay, qxl,
+                                    sizeof(QXLImageDescriptor) + sizeof(QXLQUICData) +
+                                    sizeof(QXLDataChunk));
         qxl = NULL;
         break;
     default:
-- 
2.20.1



More information about the Spice-devel mailing list