Re: [PATCH] red_parse_qxl: fix throwing away drawables that have masks

Yonit Halperin <yhalperi@xxxxxxxxxx> · Thu, 13 Dec 2012 08:29:48 -0500

On 12/13/2012 06:42 AM, Alon Levy wrote:
All looks good except the assert. We should be removing them whenever they are guest trigerable - maybe I'm not following the code, but if in red_get_image sees no pallete in the qxl struct then palette will be NULL, which means it's guest trigerable.

Hi,
The assert is for "no palette" after the image was parsed by 
red_parse_qxl. red_parse_qxl is responsible for the validity tests, so 
in the point of the assert you are referring to, it is already a server 
bug and not a guest bug.

Regards,
Yonit.

Non rgb bitmaps are allowed to not have a palette in case they
are masks (which are 1BIT bitmaps).

Related: rhbz#864982
---
  server/red_parse_qxl.c |   27 +++++++++++++--------------
  server/red_worker.c    |    8 +++-----
  2 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
index 82463e1..4b39029 100644
--- a/server/red_parse_qxl.c
+++ b/server/red_parse_qxl.c
@@ -367,7 +367,7 @@ static int bitmap_consistent(SpiceBitmap *bitmap)
  static const int BITMAP_FMT_IS_RGB[] = {0, 0, 0, 0, 0, 0, 1, 1, 1,
  1, 1};

  static SpiceImage *red_get_image(RedMemSlotInfo *slots, int
  group_id,
-                                 QXLPHYSICAL addr, uint32_t flags)
+                                 QXLPHYSICAL addr, uint32_t flags,
int is_mask)
  {
      RedDataChunk chunks;
      QXLImage *qxl;
@@ -401,7 +401,7 @@ static SpiceImage *red_get_image(RedMemSlotInfo
*slots, int group_id,
      switch (red->descriptor.type) {
      case SPICE_IMAGE_TYPE_BITMAP:
          red->u.bitmap.format = qxl->bitmap.format;
-        if (!bitmap_fmt_is_rgb(qxl->bitmap.format) &&
!qxl->bitmap.palette) {
+        if (!bitmap_fmt_is_rgb(qxl->bitmap.format) &&
!qxl->bitmap.palette && !is_mask) {
              spice_warning("guest error: missing palette on bitmap
              format=%d\n",
                            red->u.bitmap.format);
              goto error;
@@ -529,8 +529,7 @@ static void red_get_brush_ptr(RedMemSlotInfo
*slots, int group_id,
          }
          break;
      case SPICE_BRUSH_TYPE_PATTERN:
-        red->u.pattern.pat = red_get_image(slots, group_id,
qxl->u.pattern.pat, flags);
-        red_get_point_ptr(&red->u.pattern.pos, &qxl->u.pattern.pos);
+        red->u.pattern.pat = red_get_image(slots, group_id,
qxl->u.pattern.pat, flags, FALSE);
          break;
      }
  }
@@ -549,7 +548,7 @@ static void red_get_qmask_ptr(RedMemSlotInfo
*slots, int group_id,
  {
      red->flags  = qxl->flags;
      red_get_point_ptr(&red->pos, &qxl->pos);
-    red->bitmap = red_get_image(slots, group_id, qxl->bitmap,
flags);
+    red->bitmap = red_get_image(slots, group_id, qxl->bitmap, flags,
TRUE);
  }

  static void red_put_qmask(SpiceQMask *red)
@@ -574,7 +573,7 @@ static void red_put_fill(SpiceFill *red)
  static void red_get_opaque_ptr(RedMemSlotInfo *slots, int group_id,
                                 SpiceOpaque *red, QXLOpaque *qxl,
                                 uint32_t flags)
  {
-    red->src_bitmap     = red_get_image(slots, group_id,
qxl->src_bitmap, flags);
+    red->src_bitmap     = red_get_image(slots, group_id,
qxl->src_bitmap, flags, FALSE);
     red_get_rect_ptr(&red->src_area, &qxl->src_area);
     red_get_brush_ptr(slots, group_id, &red->brush, &qxl->brush,
     flags);
     red->rop_descriptor = qxl->rop_descriptor;
@@ -592,7 +591,7 @@ static void red_put_opaque(SpiceOpaque *red)
  static int red_get_copy_ptr(RedMemSlotInfo *slots, int group_id,
                              SpiceCopy *red, QXLCopy *qxl, uint32_t
                              flags)
  {
-    red->src_bitmap      = red_get_image(slots, group_id,
qxl->src_bitmap, flags);
+    red->src_bitmap      = red_get_image(slots, group_id,
qxl->src_bitmap, flags, FALSE);
      if (!red->src_bitmap) {
          return 1;
      }
@@ -612,7 +611,7 @@ static void red_put_copy(SpiceCopy *red)
  static void red_get_blend_ptr(RedMemSlotInfo *slots, int group_id,
                               SpiceBlend *red, QXLBlend *qxl,
                               uint32_t flags)
  {
-    red->src_bitmap      = red_get_image(slots, group_id,
qxl->src_bitmap, flags);
+    red->src_bitmap      = red_get_image(slots, group_id,
qxl->src_bitmap, flags, FALSE);
     red_get_rect_ptr(&red->src_area, &qxl->src_area);
     red->rop_descriptor  = qxl->rop_descriptor;
     red->scale_mode      = qxl->scale_mode;
@@ -629,7 +628,7 @@ static void
red_get_transparent_ptr(RedMemSlotInfo *slots, int group_id,
                                      SpiceTransparent *red,
                                      QXLTransparent *qxl,
                                      uint32_t flags)
  {
-    red->src_bitmap      = red_get_image(slots, group_id,
qxl->src_bitmap, flags);
+    red->src_bitmap      = red_get_image(slots, group_id,
qxl->src_bitmap, flags, FALSE);
     red_get_rect_ptr(&red->src_area, &qxl->src_area);
     red->src_color       = qxl->src_color;
     red->true_color      = qxl->true_color;
@@ -646,7 +645,7 @@ static void
red_get_alpha_blend_ptr(RedMemSlotInfo *slots, int group_id,
  {
      red->alpha_flags = qxl->alpha_flags;
      red->alpha       = qxl->alpha;
-    red->src_bitmap  = red_get_image(slots, group_id,
qxl->src_bitmap, flags);
+    red->src_bitmap  = red_get_image(slots, group_id,
qxl->src_bitmap, flags, FALSE);
      red_get_rect_ptr(&red->src_area, &qxl->src_area);
  }

@@ -655,7 +654,7 @@ static void
red_get_alpha_blend_ptr_compat(RedMemSlotInfo *slots, int group_id,
                                             uint32_t flags)
  {
      red->alpha       = qxl->alpha;
-    red->src_bitmap  = red_get_image(slots, group_id,
qxl->src_bitmap, flags);
+    red->src_bitmap  = red_get_image(slots, group_id,
qxl->src_bitmap, flags, FALSE);
      red_get_rect_ptr(&red->src_area, &qxl->src_area);
  }

@@ -690,12 +689,12 @@ static void
red_get_composite_ptr(RedMemSlotInfo *slots, int group_id,
  {
      red->flags = qxl->flags;

-    red->src_bitmap = red_get_image(slots, group_id, qxl->src,
flags);
+    red->src_bitmap = red_get_image(slots, group_id, qxl->src,
flags, FALSE);
      if (get_transform(slots, group_id, qxl->src_transform,
      &red->src_transform))
          red->flags |= SPICE_COMPOSITE_HAS_SRC_TRANSFORM;

      if (qxl->mask) {
-        red->mask_bitmap = red_get_image(slots, group_id, qxl->mask,
flags);
+        red->mask_bitmap = red_get_image(slots, group_id, qxl->mask,
flags, FALSE);
          red->flags |= SPICE_COMPOSITE_HAS_MASK;
          if (get_transform(slots, group_id, qxl->mask_transform,
          &red->mask_transform))
              red->flags |= SPICE_COMPOSITE_HAS_MASK_TRANSFORM;
@@ -718,7 +717,7 @@ static void red_put_composite(SpiceComposite
*red)
  static void red_get_rop3_ptr(RedMemSlotInfo *slots, int group_id,
                               SpiceRop3 *red, QXLRop3 *qxl, uint32_t
                               flags)
  {
-   red->src_bitmap = red_get_image(slots, group_id, qxl->src_bitmap,
flags);
+   red->src_bitmap = red_get_image(slots, group_id, qxl->src_bitmap,
flags, FALSE);
     red_get_rect_ptr(&red->src_area, &qxl->src_area);
     red_get_brush_ptr(slots, group_id, &red->brush, &qxl->brush,
     flags);
     red->rop3       = qxl->rop3;
diff --git a/server/red_worker.c b/server/red_worker.c
index 530562b..e5e3d05 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -6091,16 +6091,14 @@ static inline int
red_lz_compress_image(DisplayChannelClient *dcc,
          o_comp_data->comp_buf = lz_data->data.bufs_head;
          o_comp_data->comp_buf_size = size;
      } else {
-        if (!src->palette) {
-            spice_warning("bad guest: missing palette\n");
-            return FALSE;
-        }
+        /* masks are 1BIT bitmaps without palettes, but they are not
compressed
+         * (see fill_mask) */
+        spice_assert(src->palette);
          dest->descriptor.type = SPICE_IMAGE_TYPE_LZ_PLT;
          dest->u.lz_plt.data_size = size;
          dest->u.lz_plt.flags = src->flags &
          SPICE_BITMAP_FLAGS_TOP_DOWN;
          dest->u.lz_plt.palette = src->palette;
          dest->u.lz_plt.palette_id = src->palette->unique;
-
          o_comp_data->comp_buf = lz_data->data.bufs_head;
          o_comp_data->comp_buf_size = size;

--
1.7.7.6

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel


_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel





