[Spice-commits] 3 commits - server/red-parse-qxl.c server/tests

Frediano Ziglio fziglio at kemper.freedesktop.org
Fri Jan 8 10:19:23 PST 2016


 server/red-parse-qxl.c          |   26 +++--
 server/tests/Makefile.am        |    7 +
 server/tests/test-qxl-parsing.c |  177 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 201 insertions(+), 9 deletions(-)

New commits:
commit d3b5d2c97ecc190284e89f0dc3da820a83425593
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Thu Sep 17 14:25:34 2015 +0100

    add some tests for cursors parsing
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/server/tests/test-qxl-parsing.c b/server/tests/test-qxl-parsing.c
index 3f14a1b..d44289f 100644
--- a/server/tests/test-qxl-parsing.c
+++ b/server/tests/test-qxl-parsing.c
@@ -6,6 +6,7 @@
 #include <stdlib.h>
 #include <stdio.h>
 #include <strings.h>
+#include <unistd.h>
 #include <assert.h>
 
 #include <spice/macros.h>
@@ -28,6 +29,7 @@ test(const char *desc)
 {
     test_name = desc;
     printf("Starting test %s\n", desc);
+    alarm(5);
 }
 
 static inline QXLPHYSICAL
@@ -36,6 +38,19 @@ to_physical(const void *ptr)
     return (uintptr_t) ptr;
 }
 
+static void*
+create_chunk(size_t prefix, uint32_t size, QXLDataChunk* prev, int fill)
+{
+    uint8_t *ptr = spice_malloc0(prefix + sizeof(QXLDataChunk) + size);
+    QXLDataChunk *qxl = (QXLDataChunk *) (ptr + prefix);
+    memset(&qxl->data[0], fill, size);
+    qxl->data_size = size;
+    qxl->prev_chunk = to_physical(prev);
+    if (prev)
+        prev->next_chunk = to_physical(qxl);
+    return ptr;
+}
+
 int main(int argc, char **argv)
 {
     RedMemSlotInfo mem_info;
@@ -45,6 +60,11 @@ int main(int argc, char **argv)
     RedSurfaceCmd cmd;
     QXLSurfaceCmd qxl;
 
+    RedCursorCmd red_cursor_cmd;
+    QXLCursorCmd cursor_cmd;
+    QXLCursor *cursor;
+    QXLDataChunk *chunks[2];
+
     memset(&qxl, 0, sizeof(qxl));
 
     qxl.surface_id = 123;
@@ -82,5 +102,76 @@ int main(int argc, char **argv)
     if (!red_get_surface_cmd(&mem_info, 0, &cmd, to_physical(&qxl)))
         failure();
 
+    /* test base cursor with no problems */
+    test("base cursor command");
+    memset(&cursor_cmd, 0, sizeof(cursor_cmd));
+    cursor_cmd.type = QXL_CURSOR_SET;
+
+    cursor = create_chunk(SPICE_OFFSETOF(QXLCursor, chunk), 128 * 128 * 4, NULL, 0xaa);
+    cursor->header.unique = 1;
+    cursor->header.width = 128;
+    cursor->header.height = 128;
+    cursor->data_size = 128 * 128 * 4;
+
+    cursor_cmd.u.set.shape = to_physical(cursor);
+
+    if (red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd, to_physical(&cursor_cmd)))
+        failure();
+    free(cursor);
+
+    /* a circular list of empty chunks should not be a problems */
+    test("circular empty chunks");
+    memset(&cursor_cmd, 0, sizeof(cursor_cmd));
+    cursor_cmd.type = QXL_CURSOR_SET;
+
+    cursor = create_chunk(SPICE_OFFSETOF(QXLCursor, chunk), 0, NULL, 0xaa);
+    cursor->header.unique = 1;
+    cursor->header.width = 128;
+    cursor->header.height = 128;
+    cursor->data_size = 128 * 128 * 4;
+
+    chunks[0] = create_chunk(0, 0, &cursor->chunk, 0xaa);
+    chunks[0]->next_chunk = to_physical(&cursor->chunk);
+
+    cursor_cmd.u.set.shape = to_physical(cursor);
+
+    memset(&red_cursor_cmd, 0xaa, sizeof(red_cursor_cmd));
+    if (!red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd, to_physical(&cursor_cmd))) {
+        /* function does not return errors so there should be no data */
+        assert(red_cursor_cmd.type == QXL_CURSOR_SET);
+        assert(red_cursor_cmd.u.set.position.x == 0);
+        assert(red_cursor_cmd.u.set.position.y == 0);
+        assert(red_cursor_cmd.u.set.shape.data_size == 0);
+    }
+    free(cursor);
+    free(chunks[0]);
+
+    /* a circular list of small chunks should not be a problems */
+    test("circular small chunks");
+    memset(&cursor_cmd, 0, sizeof(cursor_cmd));
+    cursor_cmd.type = QXL_CURSOR_SET;
+
+    cursor = create_chunk(SPICE_OFFSETOF(QXLCursor, chunk), 1, NULL, 0xaa);
+    cursor->header.unique = 1;
+    cursor->header.width = 128;
+    cursor->header.height = 128;
+    cursor->data_size = 128 * 128 * 4;
+
+    chunks[0] = create_chunk(0, 1, &cursor->chunk, 0xaa);
+    chunks[0]->next_chunk = to_physical(&cursor->chunk);
+
+    cursor_cmd.u.set.shape = to_physical(cursor);
+
+    memset(&red_cursor_cmd, 0xaa, sizeof(red_cursor_cmd));
+    if (!red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd, to_physical(&cursor_cmd))) {
+        /* function does not return errors so there should be no data */
+        assert(red_cursor_cmd.type == QXL_CURSOR_SET);
+        assert(red_cursor_cmd.u.set.position.x == 0);
+        assert(red_cursor_cmd.u.set.position.y == 0);
+        assert(red_cursor_cmd.u.set.shape.data_size == 0);
+    }
+    free(cursor);
+    free(chunks[0]);
+
     return exit_code;
 }
commit 6ea345b820d8bc90d8488a9c30dccc451fdaf5c0
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Sep 8 17:59:57 2015 +0100

    add test for QXL parsing functions
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am
index fea94ed..981c570 100644
--- a/server/tests/Makefile.am
+++ b/server/tests/Makefile.am
@@ -47,6 +47,7 @@ noinst_PROGRAMS =				\
 
 TESTS =	\
 	stat_test				\
+	test-qxl-parsing			\
 	$(NULL)
 
 check_PROGRAMS = $(TESTS)
@@ -144,3 +145,9 @@ libstat_test3_a_CPPFLAGS = $(AM_CPPFLAGS) -DTEST_COMPRESS_STAT=1 -DTEST_RED_WORK
 
 libstat_test4_a_SOURCES = stat-test.c
 libstat_test4_a_CPPFLAGS = $(AM_CPPFLAGS) -DTEST_COMPRESS_STAT=1 -DTEST_RED_WORKER_STAT=1 -DTEST_NAME=stat_test4
+
+test_qxl_parsing_SOURCES =           \
+	test-qxl-parsing.c      \
+	../red-parse-qxl.c      \
+	../memslot.c            \
+	$(NULL)
diff --git a/server/tests/test-qxl-parsing.c b/server/tests/test-qxl-parsing.c
new file mode 100644
index 0000000..3f14a1b
--- /dev/null
+++ b/server/tests/test-qxl-parsing.c
@@ -0,0 +1,86 @@
+/* Do some tests on memory parsing
+ */
+
+#undef NDEBUG
+#include <string.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <strings.h>
+#include <assert.h>
+
+#include <spice/macros.h>
+#include "memslot.h"
+#include "red-parse-qxl.h"
+
+static int exit_code = EXIT_SUCCESS;
+static const char *test_name = NULL;
+
+static void
+failure(void)
+{
+    assert(test_name);
+    printf("Test %s FAILED!\n", test_name);
+    exit_code = EXIT_FAILURE;
+}
+
+static void
+test(const char *desc)
+{
+    test_name = desc;
+    printf("Starting test %s\n", desc);
+}
+
+static inline QXLPHYSICAL
+to_physical(const void *ptr)
+{
+    return (uintptr_t) ptr;
+}
+
+int main(int argc, char **argv)
+{
+    RedMemSlotInfo mem_info;
+    memslot_info_init(&mem_info, 1 /* groups */, 1 /* slots */, 1, 1, 0);
+    memslot_info_add_slot(&mem_info, 0, 0, 0 /* delta */, 0 /* start */, ~0ul /* end */, 0 /* generation */);
+
+    RedSurfaceCmd cmd;
+    QXLSurfaceCmd qxl;
+
+    memset(&qxl, 0, sizeof(qxl));
+
+    qxl.surface_id = 123;
+
+    /* try to create a surface with no issues, should succeed */
+    test("no issues");
+    qxl.u.surface_create.format = SPICE_SURFACE_FMT_32_xRGB;
+    qxl.u.surface_create.width = 128;
+    qxl.u.surface_create.stride = 512;
+    qxl.u.surface_create.height = 128;
+    qxl.u.surface_create.data = to_physical(malloc(0x10000));
+    if (red_get_surface_cmd(&mem_info, 0, &cmd, to_physical(&qxl)))
+        failure();
+
+    /* try to create a surface with a stride too small to fit
+     * the entire width.
+     * This can be used to cause buffer overflows so refuse it.
+     */
+    test("stride too small");
+    qxl.u.surface_create.stride = 256;
+    if (!red_get_surface_cmd(&mem_info, 0, &cmd, to_physical(&qxl)))
+        failure();
+
+    /* try to create a surface quite large.
+     * The sizes (width and height) were chosen so the multiplication
+     * using 32 bit values gives a very small value.
+     * These kind of values should be refused as they will cause
+     * overflows. Also the total memory for the card is not enough to
+     * hold the surface so surely can't be accepted.
+     */
+    test("too big image");
+    qxl.u.surface_create.stride = 0x08000004 * 4;
+    qxl.u.surface_create.width = 0x08000004;
+    qxl.u.surface_create.height = 0x40000020;
+    if (!red_get_surface_cmd(&mem_info, 0, &cmd, to_physical(&qxl)))
+        failure();
+
+    return exit_code;
+}
commit f84239a0358e076e2a0ff7c065a4199bd9689fae
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Wed Sep 23 15:35:19 2015 +0100

    check properly if red_get_data_chunks fails or not
    
    Instead of returning 0 which could be a valid value returns an invalid
    one and check on the caller.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index f5bdce3..dd1a09c 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -45,6 +45,8 @@ G_STATIC_ASSERT(MAX_DATA_CHUNK <= G_MAXINT32);
  */
 #define MAX_CHUNKS (MAX_DATA_CHUNK/1024u)
 
+#define INVALID_SIZE ((size_t) -1)
+
 #if 0
 static void hexdump_qxl(RedMemSlotInfo *slots, int group_id,
                         QXLPHYSICAL addr, uint8_t bytes)
@@ -120,7 +122,7 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo *slots, int group_id,
     red->prev_chunk = red->next_chunk = NULL;
     if (!memslot_validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id)) {
         red->data = NULL;
-        return 0;
+        return INVALID_SIZE;
     }
 
     while ((next_chunk = qxl->next_chunk) != 0) {
@@ -177,7 +179,7 @@ error:
     red->data_size = 0;
     red->next_chunk = NULL;
     red->data = NULL;
-    return 0;
+    return INVALID_SIZE;
 }
 
 static size_t red_get_data_chunks(RedMemSlotInfo *slots, int group_id,
@@ -189,7 +191,7 @@ static size_t red_get_data_chunks(RedMemSlotInfo *slots, int group_id,
 
     qxl = (QXLDataChunk *)memslot_get_virt(slots, addr, sizeof(*qxl), group_id, &error);
     if (error) {
-        return 0;
+        return INVALID_SIZE;
     }
     return red_get_data_chunks_ptr(slots, group_id, memslot_id, red, qxl);
 }
@@ -249,6 +251,9 @@ static SpicePath *red_get_path(RedMemSlotInfo *slots, int group_id,
     size = red_get_data_chunks_ptr(slots, group_id,
                                    memslot_get_id(slots, addr),
                                    &chunks, &qxl->chunk);
+    if (size == INVALID_SIZE) {
+        return NULL;
+    }
     data = red_linearize_chunk(&chunks, size, &free_data);
     red_put_data_chunks(&chunks);
 
@@ -328,6 +333,9 @@ static SpiceClipRects *red_get_clip_rects(RedMemSlotInfo *slots, int group_id,
     size = red_get_data_chunks_ptr(slots, group_id,
                                    memslot_get_id(slots, addr),
                                    &chunks, &qxl->chunk);
+    if (size == INVALID_SIZE) {
+        return NULL;
+    }
     data = red_linearize_chunk(&chunks, size, &free_data);
     red_put_data_chunks(&chunks);
 
@@ -530,8 +538,7 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,
         } else {
             size = red_get_data_chunks(slots, group_id,
                                        &chunks, qxl->bitmap.data);
-            spice_assert(size == bitmap_size);
-            if (size != bitmap_size) {
+            if (size == INVALID_SIZE || size != bitmap_size) {
                 red_put_data_chunks(&chunks);
                 goto error;
             }
@@ -551,8 +558,7 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,
         size = red_get_data_chunks_ptr(slots, group_id,
                                        memslot_get_id(slots, addr),
                                        &chunks, (QXLDataChunk *)qxl->quic.data);
-        spice_assert(size == red->u.quic.data_size);
-        if (size != red->u.quic.data_size) {
+        if (size == INVALID_SIZE || size != red->u.quic.data_size) {
             red_put_data_chunks(&chunks);
             goto error;
         }
@@ -870,8 +876,7 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id,
     chunk_size = red_get_data_chunks_ptr(slots, group_id,
                                          memslot_get_id(slots, addr),
                                          &chunks, &qxl->chunk);
-    if (!chunk_size) {
-        /* XXX could be a zero sized string.. */
+    if (chunk_size == INVALID_SIZE) {
         return NULL;
     }
     data = red_linearize_chunk(&chunks, chunk_size, &free_data);
@@ -1396,6 +1401,9 @@ static int red_get_cursor(RedMemSlotInfo *slots, int group_id,
     size = red_get_data_chunks_ptr(slots, group_id,
                                    memslot_get_id(slots, addr),
                                    &chunks, &qxl->chunk);
+    if (size == INVALID_SIZE) {
+        return 1;
+    }
     red->data_size = MIN(red->data_size, size);
     data = red_linearize_chunk(&chunks, size, &free_data);
     red_put_data_chunks(&chunks);


More information about the Spice-commits mailing list