>
> 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.
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel