[Spice-devel] [spice-server v2 5/9] qxl: Fix guest resources release in red_put_drawable()

[Frediano Ziglio]

> 
> At the moment, we'll unconditionally release the guest QXL resources in
> red_put_drawable() even if red_get_drawable() failed and did not
> initialize drawable->release_info_ext properly.
> This commit checks the QXLReleaseInfo in release_info_ext is non-0

You forget to update the comment

> before attempting to release it.
> 
> Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> ---
>  server/red-parse-qxl.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
> index cc6a8b51d..ccb01d92d 100644
> --- a/server/red-parse-qxl.c
> +++ b/server/red-parse-qxl.c
> @@ -1012,7 +1012,7 @@ static void red_put_clip(SpiceClip *red)
>      }
>  }
>  
> -static bool red_get_native_drawable(RedMemSlotInfo *slots, int group_id,
> +static bool red_get_native_drawable(QXLInstance *qxl_instance,
> RedMemSlotInfo *slots, int group_id,
>                                      RedDrawable *red, QXLPHYSICAL addr,
>                                      uint32_t flags)
>  {
>      QXLDrawable *qxl;
> @@ -1023,6 +1023,7 @@ static bool red_get_native_drawable(RedMemSlotInfo
> *slots, int group_id,
>      if (error) {
>          return false;
>      }
> +    red->qxl = qxl_instance;

already updated in red_get_drawable

>      red->release_info_ext.info     = &qxl->release_info;
>      red->release_info_ext.group_id = group_id;
>  
> @@ -1093,7 +1094,7 @@ static bool red_get_native_drawable(RedMemSlotInfo
> *slots, int group_id,
>      return true;
>  }
>  
> -static bool red_get_compat_drawable(RedMemSlotInfo *slots, int group_id,
> +static bool red_get_compat_drawable(QXLInstance *qxl_instance,
> RedMemSlotInfo *slots, int group_id,
>                                      RedDrawable *red, QXLPHYSICAL addr,
>                                      uint32_t flags)
>  {
>      QXLCompatDrawable *qxl;
> @@ -1103,6 +1104,7 @@ static bool red_get_compat_drawable(RedMemSlotInfo
> *slots, int group_id,
>      if (error) {
>          return false;
>      }
> +    red->qxl = qxl_instance;

here too

>      red->release_info_ext.info     = &qxl->release_info;
>      red->release_info_ext.group_id = group_id;
>  
> @@ -1176,15 +1178,16 @@ static bool red_get_compat_drawable(RedMemSlotInfo
> *slots, int group_id,
>      return true;
>  }
>  
> -static bool red_get_drawable(RedMemSlotInfo *slots, int group_id,
> +static bool red_get_drawable(QXLInstance *qxl, RedMemSlotInfo *slots, int
> group_id,
>                               RedDrawable *red, QXLPHYSICAL addr, uint32_t
>                               flags)
>  {
>      bool ret;
>  
> +    red->qxl = qxl;
>      if (flags & QXL_COMMAND_FLAG_COMPAT) {
> -        ret = red_get_compat_drawable(slots, group_id, red, addr, flags);
> +        ret = red_get_compat_drawable(qxl, slots, group_id, red, addr,
> flags);
>      } else {
> -        ret = red_get_native_drawable(slots, group_id, red, addr, flags);
> +        ret = red_get_native_drawable(qxl, slots, group_id, red, addr,
> flags);
>      }
>      return ret;
>  }
> @@ -1487,7 +1490,9 @@ void red_drawable_unref(RedDrawable *red_drawable)
>      if (--red_drawable->refs) {
>          return;
>      }
> -    red_qxl_release_resource(red_drawable->qxl,
> red_drawable->release_info_ext);
> +    if (red_drawable->qxl != NULL) {
> +        red_qxl_release_resource(red_drawable->qxl,
> red_drawable->release_info_ext);
> +    }

move in red_put_drawable ?

>      red_put_drawable(red_drawable);
>      g_free(red_drawable);
>  }
> @@ -1499,9 +1504,8 @@ RedDrawable *red_drawable_new(QXLInstance *qxl,
> RedMemSlotInfo *slots,
>      RedDrawable *red = g_new0(RedDrawable, 1);
>  
>      red->refs = 1;
> -    red->qxl = qxl;
>  
> -    if (!red_get_drawable(slots, group_id, red, addr, flags)) {
> +    if (!red_get_drawable(qxl, slots, group_id, red, addr, flags)) {
>         red_drawable_unref(red);
>         return NULL;
>      }

Frediano