[Spice-devel] [PATCH 10/11] Make sure link in RedPipeItem can be not the first field

Frediano Ziglio fziglio at redhat.com
Fri May 20 13:01:48 UTC 2016


There are many line of code were this assumption is in place.
For instance RedPipeItem* is converted to RingItem* or
viceversa.
Use SPICE_CONTAINEROF to make sure link is in RedPipeItem and the
offset is computed correctly.
Also check that RingItem pointer is checked before SPICE_CONTAINEROF.
On the other direction use &item->link instead of (RingItem*)item.
This will also make sure that code won't compile if link is
removed from RedPipeItem.

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 server/dcc-send.c      | 23 ++++++++++++-----------
 server/dcc.c           | 17 ++++++++++-------
 server/red-channel.c   |  9 ++++++---
 server/red-pipe-item.h |  2 ++
 server/reds.c          |  3 ++-
 5 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/server/dcc-send.c b/server/dcc-send.c
index 8ec22c8..44b8448 100644
--- a/server/dcc-send.c
+++ b/server/dcc-send.c
@@ -188,7 +188,11 @@ static int is_brush_lossy(RedChannelClient *rcc, SpiceBrush *brush,
 
 static RedPipeItem *dcc_get_tail(DisplayChannelClient *dcc)
 {
-    return (RedPipeItem*)ring_get_tail(&RED_CHANNEL_CLIENT(dcc)->pipe);
+    RingItem *ring_item = ring_get_tail(&RED_CHANNEL_CLIENT(dcc)->pipe);
+    if (!ring_item) {
+        return NULL;
+    }
+    return SPICE_CONTAINEROF(ring_item, RedPipeItem, link);
 }
 
 static void red_display_add_image_to_pixmap_cache(RedChannelClient *rcc,
@@ -596,16 +600,14 @@ static int pipe_rendered_drawables_intersect_with_areas(DisplayChannelClient *dc
                                                         SpiceRect *surface_areas[],
                                                         int num_surfaces)
 {
-    RedPipeItem *pipe_item;
+    RingItem *ring_item;
     Ring *pipe;
 
     spice_assert(num_surfaces);
     pipe = &RED_CHANNEL_CLIENT(dcc)->pipe;
 
-    for (pipe_item = (RedPipeItem *)ring_get_head(pipe);
-         pipe_item;
-         pipe_item = (RedPipeItem *)ring_next(pipe, &pipe_item->link))
-    {
+    RING_FOREACH(ring_item, pipe) {
+        RedPipeItem *pipe_item = SPICE_CONTAINEROF(ring_item, RedPipeItem, link);
         Drawable *drawable;
 
         if (pipe_item->type != RED_PIPE_ITEM_TYPE_DRAW)
@@ -685,7 +687,7 @@ static void red_pipe_replace_rendered_drawables_with_images(DisplayChannelClient
     int resent_surface_ids[MAX_PIPE_SIZE];
     SpiceRect resent_areas[MAX_PIPE_SIZE]; // not pointers since drawables may be released
     int num_resent;
-    RedPipeItem *pipe_item;
+    RingItem *ring_item;
     Ring *pipe;
 
     resent_surface_ids[0] = first_surface_id;
@@ -695,9 +697,8 @@ static void red_pipe_replace_rendered_drawables_with_images(DisplayChannelClient
     pipe = &RED_CHANNEL_CLIENT(dcc)->pipe;
 
     // going from the oldest to the newest
-    for (pipe_item = (RedPipeItem *)ring_get_tail(pipe);
-         pipe_item;
-         pipe_item = (RedPipeItem *)ring_prev(pipe, &pipe_item->link)) {
+    RING_FOREACH(ring_item, pipe) {
+        RedPipeItem *pipe_item = SPICE_CONTAINEROF(ring_item, RedPipeItem, link);
         Drawable *drawable;
         RedDrawablePipeItem *dpi;
         RedImageItem *image;
@@ -728,7 +729,7 @@ static void red_pipe_replace_rendered_drawables_with_images(DisplayChannelClient
 
         spice_assert(image);
         red_channel_client_pipe_remove_and_release(RED_CHANNEL_CLIENT(dcc), &dpi->dpi_pipe_item);
-        pipe_item = &image->base;
+        ring_item = &image->base.link;
     }
 }
 
diff --git a/server/dcc.c b/server/dcc.c
index 20a8f11..df5123b 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -67,6 +67,7 @@ int dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface
                                           int wait_if_used)
 {
     Ring *ring;
+    RingItem *ring_item;
     RedPipeItem *item;
     int x;
     RedChannelClient *rcc;
@@ -76,13 +77,15 @@ int dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface
        no other drawable depends on them */
 
     rcc = RED_CHANNEL_CLIENT(dcc);
-    ring = &rcc->pipe;
-    item = (RedPipeItem *) ring;
-    while ((item = (RedPipeItem *)ring_next(ring, &item->link))) {
+    ring_item = ring = &rcc->pipe;
+    item = NULL;
+    while ((ring_item = ring_next(ring, ring_item))) {
         Drawable *drawable;
         RedDrawablePipeItem *dpi = NULL;
         int depend_found = FALSE;
 
+        item = SPICE_CONTAINEROF(ring_item, RedPipeItem, link);
+
         if (item->type == RED_PIPE_ITEM_TYPE_DRAW) {
             dpi = SPICE_CONTAINEROF(item, RedDrawablePipeItem, dpi_pipe_item);
             drawable = dpi->drawable;
@@ -94,10 +97,10 @@ int dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface
 
         if (drawable->surface_id == surface_id) {
             RedPipeItem *tmp_item = item;
-            item = (RedPipeItem *)ring_prev(ring, &item->link);
+            ring_item = ring_prev(ring, &item->link);
             red_channel_client_pipe_remove_and_release(rcc, tmp_item);
-            if (!item) {
-                item = (RedPipeItem *)ring;
+            if (!ring_item) {
+                ring_item = ring;
             }
             continue;
         }
@@ -123,7 +126,7 @@ int dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface
         return TRUE;
     }
 
-    if (item) {
+    if (ring_item) {
         return red_channel_client_wait_pipe_item_sent(RED_CHANNEL_CLIENT(dcc), item,
                                                       COMMON_CLIENT_TIMEOUT);
     } else {
diff --git a/server/red-channel.c b/server/red-channel.c
index c422afd..20414f3 100644
--- a/server/red-channel.c
+++ b/server/red-channel.c
@@ -1328,13 +1328,15 @@ static inline int red_channel_client_waiting_for_ack(RedChannelClient *rcc)
 
 static inline RedPipeItem *red_channel_client_pipe_item_get(RedChannelClient *rcc)
 {
+    RingItem *ring_item;
     RedPipeItem *item;
 
     if (!rcc || rcc->send_data.blocked
              || red_channel_client_waiting_for_ack(rcc)
-             || !(item = (RedPipeItem *)ring_get_tail(&rcc->pipe))) {
+             || !(ring_item = ring_get_tail(&rcc->pipe))) {
         return NULL;
     }
+    item = SPICE_CONTAINEROF(ring_item, RedPipeItem, link);
     red_channel_client_pipe_remove(rcc, item);
     return item;
 }
@@ -1781,12 +1783,13 @@ static void red_channel_client_clear_sent_item(RedChannelClient *rcc)
 
 void red_channel_client_pipe_clear(RedChannelClient *rcc)
 {
-    RedPipeItem *item;
+    RingItem *ring_item;
 
     if (rcc) {
         red_channel_client_clear_sent_item(rcc);
     }
-    while ((item = (RedPipeItem *)ring_get_head(&rcc->pipe))) {
+    while ((ring_item = ring_get_head(&rcc->pipe))) {
+        RedPipeItem *item = SPICE_CONTAINEROF(ring_item, RedPipeItem, link);
         ring_remove(&item->link);
         red_pipe_item_unref(item);
     }
diff --git a/server/red-pipe-item.h b/server/red-pipe-item.h
index bf13b0b..4856a79 100644
--- a/server/red-pipe-item.h
+++ b/server/red-pipe-item.h
@@ -26,7 +26,9 @@ struct RedPipeItem;
 typedef void red_pipe_item_free_t(struct RedPipeItem *item);
 
 typedef struct RedPipeItem {
+    /* this MUST be the first field, there are many conversion to RingItem* */
     RingItem link;
+
     int type;
 
     /* private */
diff --git a/server/reds.c b/server/reds.c
index 90911b4..c983016 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -747,7 +747,8 @@ static void reds_agent_remove(RedsState *reds)
 
 static void vdi_port_read_buf_release(uint8_t *data, void *opaque)
 {
-    red_pipe_item_unref((RedPipeItem *)opaque);
+    RedVDIReadBuf *read_buf = (RedVDIReadBuf *)opaque;
+    red_pipe_item_unref(&read_buf->base);
 }
 
 /* returns TRUE if the buffer can be forwarded */
-- 
2.7.4



More information about the Spice-devel mailing list