> >
> > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> >
> > ---
> > server/dcc.c | 12 +++++++-----
> > 1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/server/dcc.c b/server/dcc.c
> > index bb6001e..8e25c67 100644
> > --- a/server/dcc.c
> > +++ b/server/dcc.c
> > @@ -343,7 +343,8 @@ static RedGlzDrawable
> > *get_glz_drawable(DisplayChannelClient *dcc, Drawable *dra
> > NOTE - the caller should set the glz_instance returned by the encoder
> > by
> > itself.*/
> > static GlzDrawableInstanceItem *add_glz_drawable_instance(RedGlzDrawable
> > *glz_drawable)
> > {
> > - spice_assert(glz_drawable->instances_count <
> > MAX_GLZ_DRAWABLE_INSTANCES);
> > + spice_return_val_if_fail(glz_drawable->instances_count <
> > MAX_GLZ_DRAWABLE_INSTANCES, NULL);
> > +
> > // NOTE: We assume the additions are performed consecutively, without
> > removals in the middle
> > GlzDrawableInstanceItem *ret = glz_drawable->instances_pool +
> > glz_drawable->instances_count;
> > glz_drawable->instances_count++;
>
> This will lead to a core on caller. Better the assert which stop with a
> proper error.
>
> > @@ -363,11 +364,12 @@ int dcc_compress_image_glz(DisplayChannelClient *dcc,
> > SpiceImage *dest, SpiceBitmap *src, Drawable
> > *drawable,
> > compress_send_data_t* o_comp_data)
> > {
> > + spice_return_val_if_fail(bitmap_fmt_is_rgb(src->format), FALSE);
> > +
> > DisplayChannel *display_channel = DCC_TO_DC(dcc);
> > #ifdef COMPRESS_STAT
> > stat_time_t start_time = stat_now(display_channel->glz_stat.clock);
> > #endif
> > - spice_assert(bitmap_fmt_is_rgb(src->format));
> > GlzData *glz_data = &dcc->glz_data;
> > ZlibData *zlib_data;
> > LzImageType type = MAP_BITMAP_FMT_TO_LZ_IMAGE_TYPE[src->format];
> > @@ -498,7 +500,7 @@ int dcc_compress_image_lz(DisplayChannelClient *dcc,
> > } else {
> > /* masks are 1BIT bitmaps without palettes, but they are not
> > compressed
> > * (see fill_mask) */
> > - spice_assert(src->palette);
> > + spice_return_val_if_fail(src->palette, FALSE);
> > 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;
>
> These would make sense if upper layer could deal with it.
> Actually is not!
> When dcc_compress_image is called is expected to have the compressed image
> set
> but in these cases image is not initialized properly causing potentially
> crashes or leaking of information.
>
> > @@ -976,7 +978,7 @@ int dcc_pixmap_cache_unlocked_add(DisplayChannelClient
> > *dcc, uint64_t id,
> > uint64_t serial;
> > int key;
> >
> > - spice_assert(size > 0);
> > + spice_return_val_if_fail(size > 0, FALSE);
> >
> > item = spice_new(NewCacheItem, 1);
> > serial =
> > red_channel_client_get_message_serial(RED_CHANNEL_CLIENT(dcc));
>
> This could make sense but if spice-server accept an image where width *
> height == 0
> (size is unsigned so this condition is the same as size == 0) there should be
> no
> warning/error/abortion, if it does not accept such images (which I think is
> the case)
> it means a corruption happen and assert is perfectly fine.
>
> > @@ -1005,7 +1007,7 @@ int
> > dcc_pixmap_cache_unlocked_add(DisplayChannelClient
> > *dcc, uint64_t id,
> >
> > now = &cache->hash_table[BITS_CACHE_HASH_KEY(tail->id)];
> > for (;;) {
> > - spice_assert(*now);
> > + spice_return_val_if_fail(*now, FALSE);
> > if (*now == tail) {
> > *now = tail->next;
> > break;
>
> This is a ring, if you get a NULL there is a memory/structure corruption so
> assert is fine.
>
Rejected
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel