[Spice-devel] [RFC v1 1/4] red-parse-qxl: Extract the dmabuf fd from the scanout

[Frediano Ziglio]

Il giorno mer 11 gen 2023 alle ore 05:42 Vivek Kasireddy
<vivek.kasireddy at intel.com> ha scritto:
>
> If the scanout has a valid dmabuf fd, then it is extracted and a
> copy (of the fd) is stored in the drawable.
>
> Cc: Gerd Hoffmann <kraxel at redhat.com>
> Cc: Marc-André Lureau <marcandre.lureau at redhat.com>
> Cc: Dongwon Kim <dongwon.kim at intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy at intel.com>
> ---
>  server/red-parse-qxl.cpp | 8 ++++++++
>  server/red-parse-qxl.h   | 1 +
>  2 files changed, 9 insertions(+)
>
> diff --git a/server/red-parse-qxl.cpp b/server/red-parse-qxl.cpp
> index 68b9759d..8d9b82fb 100644
> --- a/server/red-parse-qxl.cpp
> +++ b/server/red-parse-qxl.cpp
> @@ -1038,6 +1038,7 @@ static bool red_get_native_drawable(QXLInstance *qxl_instance, RedMemSlotInfo *s
>                                      RedDrawable *red, QXLPHYSICAL addr, uint32_t flags)
>  {
>      QXLDrawable *qxl;
> +    SpiceMsgDisplayGlScanoutUnix *scanout;
>      int i;
>
>      qxl = static_cast<QXLDrawable *>(memslot_get_virt(slots, addr, sizeof(*qxl), group_id));
> @@ -1059,6 +1060,13 @@ static bool red_get_native_drawable(QXLInstance *qxl_instance, RedMemSlotInfo *s
>          red_get_rect_ptr(&red->surfaces_rects[i], &qxl->surfaces_rects[i]);
>      }
>
> +    red->dmabuf_fd = 0;

0 is a perfectly valid file descriptor, usually invalid file
descriptors are initialized with -1.

> +    scanout = red_qxl_get_gl_scanout(qxl_instance);
> +    if (scanout != nullptr) {
> +        red->dmabuf_fd = scanout->drm_dma_buf_fd;
> +    }
> +    red_qxl_put_gl_scanout(qxl_instance, scanout);
> +
>      red->type = qxl->type;
>      switch (red->type) {
>      case QXL_DRAW_ALPHA_BLEND:
> diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
> index 8bf0e2e3..dee50743 100644
> --- a/server/red-parse-qxl.h
> +++ b/server/red-parse-qxl.h
> @@ -67,6 +67,7 @@ struct RedDrawable final: public RedQXLResource<RedDrawable> {
>          SpiceWhiteness whiteness;
>          SpiceComposite composite;
>      } u;
> +    int32_t dmabuf_fd;
>  };
>
>  struct RedUpdateCmd final: public RedQXLResource<RedUpdateCmd> {

This patch looks pretty error prone, it's easy to generate leaks or
use after free (of file descriptor).
Also it adds the assumption that the drawing is always associated with
the DMA surface which is racy.

Frediano