[Spice-devel] [PATCH] replay: Handle errors in record file

Frediano Ziglio fziglio at redhat.com
Thu Jun 9 09:36:12 UTC 2016


Detect errors in record file. This can happen from a wrong version or
corruption of files.
Allocations are kept into a GList to be able to free in case some
errors happened.
To check fscanf read all needed information a dummy "%n" is appended
to any string and the value stored there is tested. This as scanf family
could return a valid value but not entirely process the string so
adding a "%n" and checking this was processed make sure all expected
string is found.
The "eof" field is used to mark any error happened, so in most cases
there is no explicit check beside when this could cause a problem
(for instance results of replay_fscanf used which would result in
uninitialised variable usage).

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

diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index b17c38b..f916fa8 100644
--- a/server/red-replay-qxl.c
+++ b/server/red-replay-qxl.c
@@ -49,29 +49,33 @@ struct SpiceReplay {
     GArray *id_map_inv; // replay id -> record id
     GArray *id_free; // free list
     int nsurfaces;
+    int end_pos;
+
+    GList *allocated;
 
     pthread_mutex_t mutex;
     pthread_cond_t cond;
 };
 
-static int replay_fread(SpiceReplay *replay, uint8_t *buf, size_t size)
+static ssize_t replay_fread(SpiceReplay *replay, uint8_t *buf, size_t size)
 {
-    if (replay->eof) {
-        return 0;
-    }
-    if (feof(replay->fd)) {
+    if (replay->eof
+        || feof(replay->fd)
+        || fread(buf, 1, size, replay->fd) != size) {
         replay->eof = 1;
         return 0;
     }
-    return fread(buf, size, 1, replay->fd);
+    return size;
 }
 
 __attribute__((format(scanf, 2, 3)))
-static replay_t replay_fscanf(SpiceReplay *replay, const char *fmt, ...)
+static replay_t replay_fscanf_check(SpiceReplay *replay, const char *fmt, ...)
 {
     va_list ap;
     int ret;
 
+    replay->end_pos = -1;
+
     if (replay->eof) {
         return REPLAY_EOF;
     }
@@ -82,11 +86,27 @@ static replay_t replay_fscanf(SpiceReplay *replay, const char *fmt, ...)
     va_start(ap, fmt);
     ret = vfscanf(replay->fd, fmt, ap);
     va_end(ap);
-    if (ret == EOF) {
+    if (ret == EOF || replay->end_pos < 0) {
         replay->eof = 1;
     }
     return replay->eof ? REPLAY_EOF : REPLAY_OK;
 }
+#define replay_fscanf(r, fmt, ...) \
+    replay_fscanf_check(r, fmt "%n", ## __VA_ARGS__, &r->end_pos)
+
+static inline void *replay_malloc(SpiceReplay *replay, size_t size)
+{
+    void *p = spice_malloc(size);
+    replay->allocated = g_list_prepend(replay->allocated, p);
+    return p;
+}
+
+static inline void *replay_malloc0(SpiceReplay *replay, size_t size)
+{
+    void *p = spice_malloc0(size);
+    replay->allocated = g_list_prepend(replay->allocated, p);
+    return p;
+}
 
 static uint32_t replay_id_get(SpiceReplay *replay, uint32_t id)
 {
@@ -188,18 +208,20 @@ static void hexdump(uint8_t *hex, uint8_t bytes)
 static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *size, uint8_t
                             **buf, size_t base_size)
 {
-    char template[1024];
-    int with_zlib = -1;
-    int zlib_size;
-    uint8_t *zlib_buffer;
+    char file_prefix[1024];
+    int with_zlib;
     z_stream strm;
 
-    snprintf(template, sizeof(template), "binary %%d %s %%ld:", prefix);
-    if (replay_fscanf(replay, template, &with_zlib, size) == REPLAY_EOF)
+    if (replay_fscanf(replay, "binary %d %1000s %ld:", &with_zlib, file_prefix, size) == REPLAY_EOF) {
         return REPLAY_EOF;
+    }
+    if (strcmp(file_prefix, prefix) != 0) {
+        replay->eof = 1;
+        return REPLAY_EOF;
+    }
 
     if (*buf == NULL) {
-        *buf = spice_malloc(*size + base_size);
+        *buf = replay_malloc(replay, *size + base_size);
     }
 #if 0
     {
@@ -211,10 +233,18 @@ static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *siz
     spice_return_val_if_fail(with_zlib != -1, REPLAY_EOF);
     if (with_zlib) {
         int ret;
+        GList *elem;
+        uint8_t *zlib_buffer;
+        int zlib_size;
 
-        replay_fscanf(replay, "%d:", &zlib_size);
-        zlib_buffer = spice_malloc(zlib_size);
-        replay_fread(replay, zlib_buffer, zlib_size);
+        if (replay_fscanf(replay, "%d:", &zlib_size) == REPLAY_EOF) {
+            return REPLAY_EOF;
+        }
+        zlib_buffer = replay_malloc(replay, zlib_size);
+        elem = replay->allocated;
+        if (replay_fread(replay, zlib_buffer, zlib_size) != zlib_size) {
+            return REPLAY_EOF;
+        }
         strm.zalloc = Z_NULL;
         strm.zfree = Z_NULL;
         strm.opaque = Z_NULL;
@@ -241,16 +271,16 @@ static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *siz
             }
         }
         (void)inflateEnd(&strm);
+        replay->allocated = g_list_remove_link(replay->allocated, elem);
         free(zlib_buffer); // TODO - avoid repeat alloc/dealloc by keeping last
     } else {
         replay_fread(replay, *buf + base_size, *size);
     }
 #endif
-    replay_fscanf(replay, "\n");
-    return REPLAY_OK;
+    return replay_fscanf(replay, "\n");
 }
 
-static size_t red_replay_data_chunks(SpiceReplay *replay, const char *prefix,
+static ssize_t red_replay_data_chunks(SpiceReplay *replay, const char *prefix,
                                      uint8_t **mem, size_t base_size)
 {
     size_t data_size;
@@ -258,7 +288,9 @@ static size_t red_replay_data_chunks(SpiceReplay *replay, const char *prefix,
     size_t next_data_size;
     QXLDataChunk *cur, *next;
 
-    replay_fscanf(replay, "data_chunks %d %zu\n", &count_chunks, &data_size);
+    if (replay_fscanf(replay, "data_chunks %d %zu\n", &count_chunks, &data_size) == REPLAY_EOF) {
+        return REPLAY_EOF;
+    }
     if (base_size == 0) {
         base_size = sizeof(QXLDataChunk);
     }
@@ -316,19 +348,26 @@ static void red_replay_point16_ptr(SpiceReplay *replay, QXLPoint16 *qxl)
 
 static void red_replay_rect_ptr(SpiceReplay *replay, const char *prefix, QXLRect *qxl)
 {
-    char template[1024];
+    char file_prefix[1024];
 
-    snprintf(template, sizeof(template), "rect %s %%d %%d %%d %%d\n", prefix);
-    replay_fscanf(replay, template, &qxl->top, &qxl->left, &qxl->bottom, &qxl->right);
+    if (replay_fscanf(replay, "rect %1000s %d %d %d %d\n", file_prefix, &qxl->top, &qxl->left,
+                      &qxl->bottom, &qxl->right) == REPLAY_EOF) {
+        return;
+    }
+    if (strcmp(file_prefix, prefix) != 0) {
+        replay->eof = 1;
+    }
 }
 
 static QXLPath *red_replay_path(SpiceReplay *replay)
 {
     QXLPath *qxl = NULL;
-    size_t data_size;
+    ssize_t data_size;
 
     data_size = red_replay_data_chunks(replay, "path", (uint8_t**)&qxl, sizeof(QXLPath));
-    qxl->data_size = data_size;
+    if (data_size >= 0) {
+        qxl->data_size = data_size;
+    }
     return qxl;
 }
 
@@ -344,9 +383,11 @@ static QXLClipRects *red_replay_clip_rects(SpiceReplay *replay)
     QXLClipRects *qxl = NULL;
     int num_rects;
 
-    replay_fscanf(replay, "num_rects %d\n", &num_rects);
-    red_replay_data_chunks(replay, "clip_rects", (uint8_t**)&qxl, sizeof(QXLClipRects));
-    qxl->num_rects = num_rects;
+    if (replay_fscanf(replay, "num_rects %d\n", &num_rects) == REPLAY_EOF) {
+        return NULL;
+    }
+    if (red_replay_data_chunks(replay, "clip_rects", (uint8_t**)&qxl, sizeof(QXLClipRects)) >= 0)
+        qxl->num_rects = num_rects;
     return qxl;
 }
 
@@ -365,24 +406,29 @@ static uint8_t *red_replay_image_data_flat(SpiceReplay *replay, size_t *size)
 
 static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
 {
-    QXLImage* qxl = NULL;
-    size_t bitmap_size, size;
+    QXLImage *qxl = NULL;
+    size_t bitmap_size;
+    ssize_t size;
     uint8_t qxl_flags;
     int temp;
     int has_palette;
     int has_image;
 
-    replay_fscanf(replay, "image %d\n", &has_image);
+    if (replay_fscanf(replay, "image %d\n", &has_image) == REPLAY_EOF) {
+        return NULL;
+    }
     if (!has_image) {
         return NULL;
     }
 
-    qxl = spice_new0(QXLImage, 1);
+    qxl = (QXLImage*)replay_malloc0(replay, sizeof(QXLImage));
     replay_fscanf(replay, "descriptor.id %"PRIu64"\n", &qxl->descriptor.id);
     replay_fscanf(replay, "descriptor.type %d\n", &temp); qxl->descriptor.type = temp;
     replay_fscanf(replay, "descriptor.flags %d\n", &temp); qxl->descriptor.flags = temp;
     replay_fscanf(replay, "descriptor.width %d\n", &qxl->descriptor.width);
-    replay_fscanf(replay, "descriptor.height %d\n", &qxl->descriptor.height);
+    if (replay_fscanf(replay, "descriptor.height %d\n", &qxl->descriptor.height) == REPLAY_EOF) {
+        return NULL;
+    }
 
     switch (qxl->descriptor.type) {
     case SPICE_IMAGE_TYPE_BITMAP:
@@ -397,8 +443,10 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
             QXLPalette *qp;
             int i, num_ents;
 
-            replay_fscanf(replay, "qp.num_ents %d\n", &num_ents);
-            qp = spice_malloc(sizeof(QXLPalette) + num_ents * sizeof(qp->ents[0]));
+            if (replay_fscanf(replay, "qp.num_ents %d\n", &num_ents) == REPLAY_EOF) {
+                return NULL;
+            }
+            qp = replay_malloc(replay, sizeof(QXLPalette) + num_ents * sizeof(qp->ents[0]));
             qp->num_ents = num_ents;
             qxl->bitmap.palette = QXLPHYSICAL_FROM_PTR(qp);
             replay_fscanf(replay, "unique %"PRIu64"\n", &qp->unique);
@@ -413,7 +461,7 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
         if (qxl_flags & QXL_BITMAP_DIRECT) {
             qxl->bitmap.data = QXLPHYSICAL_FROM_PTR(red_replay_image_data_flat(replay, &bitmap_size));
         } else {
-            size = red_replay_data_chunks(replay, "bitmap.data", (uint8_t**)&qxl->bitmap.data, 0);
+            ssize_t size = red_replay_data_chunks(replay, "bitmap.data", (uint8_t**)&qxl->bitmap.data, 0);
             if (size != bitmap_size) {
                 spice_printerr("bad image, %zu != %zu", size, bitmap_size);
                 return NULL;
@@ -421,13 +469,18 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
         }
         break;
     case SPICE_IMAGE_TYPE_SURFACE:
-        replay_fscanf(replay, "surface_image.surface_id %d\n", &qxl->surface_image.surface_id);
+        if (replay_fscanf(replay, "surface_image.surface_id %d\n", &qxl->surface_image.surface_id)
+            == REPLAY_EOF) {
+            return NULL;
+        }
         qxl->surface_image.surface_id = replay_id_get(replay, qxl->surface_image.surface_id);
         break;
     case SPICE_IMAGE_TYPE_QUIC:
         // TODO - make this much nicer (precompute size and allocs, store them during
         // record, then reread into them. and use MPEG-4).
-        replay_fscanf(replay, "quic.data_size %d\n", &qxl->quic.data_size);
+        if (replay_fscanf(replay, "quic.data_size %d\n", &qxl->quic.data_size) == REPLAY_EOF) {
+            return NULL;
+        }
         qxl = realloc(qxl, sizeof(QXLImageDescriptor) + sizeof(QXLQUICData) +
                       qxl->quic.data_size);
         size = red_replay_data_chunks(replay, "quic.data", (uint8_t**)&qxl->quic.data, 0);
@@ -468,7 +521,10 @@ static void red_replay_image_free(SpiceReplay *replay, QXLPHYSICAL p, uint32_t f
 
 static void red_replay_brush_ptr(SpiceReplay *replay, QXLBrush *qxl, uint32_t flags)
 {
-    replay_fscanf(replay, "type %d\n", &qxl->type);
+    if (replay_fscanf(replay, "type %d\n", &qxl->type) == REPLAY_EOF) {
+        return;
+    }
+
     switch (qxl->type) {
     case SPICE_BRUSH_TYPE_SOLID:
         replay_fscanf(replay, "u.color %d\n", &qxl->u.color);
@@ -632,7 +688,10 @@ static void red_replay_stroke_ptr(SpiceReplay *replay, QXLStroke *qxl, uint32_t
     int temp;
 
     qxl->path = QXLPHYSICAL_FROM_PTR(red_replay_path(replay));
-    replay_fscanf(replay, "attr.flags %d\n", &temp); qxl->attr.flags = temp;
+    if (replay_fscanf(replay, "attr.flags %d\n", &temp) == REPLAY_EOF) {
+        return;
+    }
+    qxl->attr.flags = temp;
     if (qxl->attr.flags & SPICE_LINE_FLAGS_STYLED) {
         size_t size;
 
@@ -659,17 +718,19 @@ static QXLString *red_replay_string(SpiceReplay *replay)
     uint32_t data_size;
     uint16_t length;
     uint16_t flags;
-    size_t chunk_size;
+    ssize_t chunk_size;
     QXLString *qxl = NULL;
 
     replay_fscanf(replay, "data_size %d\n", &data_size);
     replay_fscanf(replay, "length %d\n", &temp); length = temp;
     replay_fscanf(replay, "flags %d\n", &temp); flags = temp;
     chunk_size = red_replay_data_chunks(replay, "string", (uint8_t**)&qxl, sizeof(QXLString));
-    qxl->data_size = data_size;
-    qxl->length = length;
-    qxl->flags = flags;
-    spice_assert(chunk_size == qxl->data_size);
+    if (chunk_size >= 0) {
+        qxl->data_size = data_size;
+        qxl->length = length;
+        qxl->flags = flags;
+        spice_assert(chunk_size == qxl->data_size);
+    }
     return qxl;
 }
 
@@ -729,7 +790,9 @@ static void red_replay_invers_free(SpiceReplay *replay, QXLInvers *qxl, uint32_t
 
 static void red_replay_clip_ptr(SpiceReplay *replay, QXLClip *qxl)
 {
-    replay_fscanf(replay, "type %d\n", &qxl->type);
+    if (replay_fscanf(replay, "type %d\n", &qxl->type) == REPLAY_EOF) {
+        return;
+    }
     switch (qxl->type) {
     case SPICE_CLIP_TYPE_RECTS:
         qxl->data = QXLPHYSICAL_FROM_PTR(red_replay_clip_rects(replay));
@@ -759,7 +822,7 @@ static uint8_t *red_replay_transform(SpiceReplay *replay)
 
 static void red_replay_composite_ptr(SpiceReplay *replay, QXLComposite *qxl, uint32_t flags)
 {
-    int enabled;
+    int enabled = 0;
 
     replay_fscanf(replay, "flags %d\n", &qxl->flags);
     qxl->src = QXLPHYSICAL_FROM_PTR(red_replay_image(replay, flags));
@@ -788,7 +851,7 @@ static void red_replay_composite_free(SpiceReplay *replay, QXLComposite *qxl, ui
 
 static QXLDrawable *red_replay_native_drawable(SpiceReplay *replay, uint32_t flags)
 {
-    QXLDrawable *qxl = spice_malloc0(sizeof(QXLDrawable)); // TODO - this is too large usually
+    QXLDrawable *qxl = replay_malloc0(replay, sizeof(QXLDrawable)); // TODO - this is too large usually
     int i;
     int temp;
 
@@ -798,16 +861,22 @@ static QXLDrawable *red_replay_native_drawable(SpiceReplay *replay, uint32_t fla
     replay_fscanf(replay, "mm_time %d\n", &qxl->mm_time);
     replay_fscanf(replay, "self_bitmap %d\n", &temp); qxl->self_bitmap = temp;
     red_replay_rect_ptr(replay, "self_bitmap_area", &qxl->self_bitmap_area);
-    replay_fscanf(replay, "surface_id %d\n", &qxl->surface_id);
+    if (replay_fscanf(replay, "surface_id %d\n", &qxl->surface_id) == REPLAY_EOF) {
+        return NULL;
+    }
     qxl->surface_id = replay_id_get(replay, qxl->surface_id);
 
     for (i = 0; i < 3; i++) {
-        replay_fscanf(replay, "surfaces_dest %d\n", &qxl->surfaces_dest[i]);
+        if (replay_fscanf(replay, "surfaces_dest %d\n", &qxl->surfaces_dest[i]) == REPLAY_EOF) {
+            return NULL;
+        }
         qxl->surfaces_dest[i] = replay_id_get(replay, qxl->surfaces_dest[i]);
         red_replay_rect_ptr(replay, "surfaces_rects", &qxl->surfaces_rects[i]);
     }
 
-    replay_fscanf(replay, "type %d\n", &temp); qxl->type = temp;
+    if (replay_fscanf(replay, "type %d\n", &temp) == REPLAY_EOF)
+        return NULL;
+    qxl->type = temp;
     switch (qxl->type) {
     case QXL_DRAW_ALPHA_BLEND:
         red_replay_alpha_blend_ptr(replay, &qxl->u.alpha_blend, flags);
@@ -857,7 +926,11 @@ static QXLDrawable *red_replay_native_drawable(SpiceReplay *replay, uint32_t fla
         spice_warn_if_reached();
         break;
     };
-    return qxl;
+    if (!replay->eof) {
+        return qxl;
+    }
+
+    return NULL;
 }
 
 static void red_replay_native_drawable_free(SpiceReplay *replay, QXLDrawable *qxl, uint32_t flags)
@@ -919,7 +992,7 @@ static void red_replay_native_drawable_free(SpiceReplay *replay, QXLDrawable *qx
 static QXLCompatDrawable *red_replay_compat_drawable(SpiceReplay *replay, uint32_t flags)
 {
     int temp;
-    QXLCompatDrawable *qxl = spice_malloc0(sizeof(QXLCompatDrawable)); // TODO - too large usually
+    QXLCompatDrawable *qxl = replay_malloc0(replay, sizeof(QXLCompatDrawable)); // TODO - too large usually
 
     red_replay_rect_ptr(replay, "bbox", &qxl->bbox);
     red_replay_clip_ptr(replay, &qxl->clip);
@@ -929,7 +1002,10 @@ static QXLCompatDrawable *red_replay_compat_drawable(SpiceReplay *replay, uint32
     replay_fscanf(replay, "bitmap_offset %d\n", &temp); qxl->bitmap_offset = temp;
     red_replay_rect_ptr(replay, "bitmap_area", &qxl->bitmap_area);
 
-    replay_fscanf(replay, "type %d\n", &temp); qxl->type = temp;
+    if (replay_fscanf(replay, "type %d\n", &temp) == REPLAY_EOF) {
+        return NULL;
+    }
+    qxl->type = temp;
     switch (qxl->type) {
     case QXL_DRAW_ALPHA_BLEND:
         red_replay_alpha_blend_ptr_compat(replay, &qxl->u.alpha_blend, flags);
@@ -976,14 +1052,15 @@ static QXLCompatDrawable *red_replay_compat_drawable(SpiceReplay *replay, uint32
         spice_error("%s: unknown type %d", __FUNCTION__, qxl->type);
         break;
     };
-    return qxl;
+    if (!replay->eof) {
+        return qxl;
+    }
+
+    return NULL;
 }
 
 static QXLPHYSICAL red_replay_drawable(SpiceReplay *replay, uint32_t flags)
 {
-    if (replay->eof) {
-        return 0;
-    }
     replay_fscanf(replay, "drawable\n");
     if (flags & QXL_COMMAND_FLAG_COMPAT) {
         return QXLPHYSICAL_FROM_PTR(red_replay_compat_drawable(replay, flags));
@@ -994,12 +1071,14 @@ static QXLPHYSICAL red_replay_drawable(SpiceReplay *replay, uint32_t flags)
 
 static QXLUpdateCmd *red_replay_update_cmd(SpiceReplay *replay)
 {
-    QXLUpdateCmd *qxl = spice_malloc0(sizeof(QXLUpdateCmd));
+    QXLUpdateCmd *qxl = replay_malloc0(replay, sizeof(QXLUpdateCmd));
 
     replay_fscanf(replay, "update\n");
     red_replay_rect_ptr(replay, "area", &qxl->area);
     replay_fscanf(replay, "update_id %d\n", &qxl->update_id);
-    replay_fscanf(replay, "surface_id %d\n", &qxl->surface_id);
+    if (replay_fscanf(replay, "surface_id %d\n", &qxl->surface_id) == REPLAY_EOF) {
+        return NULL;
+    }
     qxl->surface_id = replay_id_get(replay, qxl->surface_id);
 
     return qxl;
@@ -1024,14 +1103,19 @@ static QXLSurfaceCmd *red_replay_surface_cmd(SpiceReplay *replay)
     replay_fscanf(replay, "surface_cmd\n");
     replay_fscanf(replay, "surface_id %d\n", &qxl->surface_id);
     replay_fscanf(replay, "type %d\n", &temp); qxl->type = temp;
-    replay_fscanf(replay, "flags %d\n", &qxl->flags);
+    if (replay_fscanf(replay, "flags %d\n", &qxl->flags) == REPLAY_EOF) {
+        return NULL;
+    }
 
     switch (qxl->type) {
     case QXL_SURFACE_CMD_CREATE:
         replay_fscanf(replay, "u.surface_create.format %d\n", &qxl->u.surface_create.format);
         replay_fscanf(replay, "u.surface_create.width %d\n", &qxl->u.surface_create.width);
         replay_fscanf(replay, "u.surface_create.height %d\n", &qxl->u.surface_create.height);
-        replay_fscanf(replay, "u.surface_create.stride %d\n", &qxl->u.surface_create.stride);
+        if (replay_fscanf(replay, "u.surface_create.stride %d\n", &qxl->u.surface_create.stride)
+            == REPLAY_EOF) {
+            return NULL;
+        }
         size = qxl->u.surface_create.height * abs(qxl->u.surface_create.stride);
         if ((qxl->flags & QXL_SURF_FLAG_KEEP_DATA) != 0) {
             read_binary(replay, "data", &read_size, (uint8_t**)&qxl->u.surface_create.data, 0);
@@ -1039,7 +1123,7 @@ static QXLSurfaceCmd *red_replay_surface_cmd(SpiceReplay *replay)
                 spice_printerr("mismatch %zu != %zu", size, read_size);
             }
         } else {
-            qxl->u.surface_create.data = QXLPHYSICAL_FROM_PTR(spice_malloc(size));
+            qxl->u.surface_create.data = QXLPHYSICAL_FROM_PTR(replay_malloc(replay, size));
         }
         qxl->surface_id = replay_id_new(replay, qxl->surface_id);
         break;
@@ -1077,7 +1161,9 @@ static QXLCursor *red_replay_cursor(SpiceReplay *replay)
     replay_fscanf(replay, "header.hot_spot_y %d\n", &temp);
     cursor.header.hot_spot_y = temp;
 
-    replay_fscanf(replay, "data_size %d\n", &temp);
+    if (replay_fscanf(replay, "data_size %d\n", &temp) == REPLAY_EOF) {
+        return NULL;
+    }
     cursor.data_size = temp;
     cursor.data_size = red_replay_data_chunks(replay, "cursor", (uint8_t**)&qxl, sizeof(QXLCursor));
     qxl->header = cursor.header;
@@ -1091,7 +1177,9 @@ static QXLCursorCmd *red_replay_cursor_cmd(SpiceReplay *replay)
     QXLCursorCmd *qxl = spice_new0(QXLCursorCmd, 1);
 
     replay_fscanf(replay, "cursor_cmd\n");
-    replay_fscanf(replay, "type %d\n", &temp);
+    if (replay_fscanf(replay, "type %d\n", &temp) == REPLAY_EOF) {
+        return NULL;
+    }
     qxl->type = temp;
     switch (qxl->type) {
     case QXL_CURSOR_SET:
@@ -1139,8 +1227,10 @@ static void replay_handle_create_primary(QXLWorker *worker, SpiceReplay *replay)
 
     replay_fscanf(replay, "%d %d %d %d\n", &surface.width, &surface.height,
         &surface.stride, &surface.format);
-    replay_fscanf(replay, "%d %d %d %d\n", &surface.position, &surface.mouse_mode,
-        &surface.flags, &surface.type);
+    if (replay_fscanf(replay, "%d %d %d %d\n", &surface.position, &surface.mouse_mode,
+        &surface.flags, &surface.type) == REPLAY_EOF) {
+        return;
+    }
     read_binary(replay, "data", &size, &mem, 0);
     surface.group_id = 0;
     surface.mem = QXLPHYSICAL_FROM_PTR(mem);
@@ -1185,7 +1275,7 @@ static void replay_handle_dev_input(QXLWorker *worker, SpiceReplay *replay,
 SPICE_GNUC_VISIBLE QXLCommandExt* spice_replay_next_cmd(SpiceReplay *replay,
                                                          QXLWorker *worker)
 {
-    QXLCommandExt* cmd;
+    QXLCommandExt* cmd = NULL;
     uint64_t timestamp;
     int type;
     int what = -1;
@@ -1195,7 +1285,7 @@ SPICE_GNUC_VISIBLE QXLCommandExt* spice_replay_next_cmd(SpiceReplay *replay,
         replay_fscanf(replay, "event %d %d %d %"PRIu64"\n", &counter,
                             &what, &type, &timestamp);
         if (replay->eof) {
-            return NULL;
+            goto eof;
         }
         if (what == 1) {
             replay_handle_dev_input(worker, replay, type);
@@ -1224,6 +1314,10 @@ SPICE_GNUC_VISIBLE QXLCommandExt* spice_replay_next_cmd(SpiceReplay *replay,
         break;
     }
 
+    if (replay->eof) {
+        goto eof;
+    }
+
     QXLReleaseInfo *info;
     switch (cmd->cmd.type) {
     case QXL_CMD_DRAW:
@@ -1234,9 +1328,27 @@ SPICE_GNUC_VISIBLE QXLCommandExt* spice_replay_next_cmd(SpiceReplay *replay,
         info->id = (uintptr_t)cmd;
     }
 
+    /* all buffer allocated will be used by the caller but
+     * free the list of buffer allocated to avoid to free on next calls */
+    if (replay->allocated) {
+        g_list_free(replay->allocated);
+        replay->allocated = NULL;
+    }
+
     replay->counter++;
 
     return cmd;
+
+eof:
+    /* if there were some allocation during the read free all
+     * buffers allocated to avoid leaks */
+    if (replay->allocated) {
+        g_list_free_full(replay->allocated, free);
+        replay->allocated = NULL;
+    }
+    if (cmd)
+        g_free(cmd);
+    return NULL;
 }
 
 SPICE_GNUC_VISIBLE void spice_replay_free_cmd(SpiceReplay *replay, QXLCommandExt *cmd)
@@ -1305,6 +1417,7 @@ SpiceReplay *spice_replay_new(FILE *file, int nsurfaces)
     replay->id_map_inv = g_array_new(FALSE, FALSE, sizeof(uint32_t));
     replay->id_free = g_array_new(FALSE, FALSE, sizeof(uint32_t));
     replay->nsurfaces = nsurfaces;
+    replay->allocated = NULL;
 
     /* reserve id 0 */
     replay_id_new(replay, 0);
@@ -1316,6 +1429,7 @@ SPICE_GNUC_VISIBLE void spice_replay_free(SpiceReplay *replay)
 {
     spice_return_if_fail(replay != NULL);
 
+    g_list_free_full(replay->allocated, free);
     pthread_mutex_destroy(&replay->mutex);
     pthread_cond_destroy(&replay->cond);
     g_array_free(replay->id_map, TRUE);
-- 
2.7.4



More information about the Spice-devel mailing list