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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Il giorno mer 11 gen 2023 alle ore 05:42 Vivek Kasireddy
<vivek.kasireddy@xxxxxxxxx> 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@xxxxxxxxxx>
> Cc: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> Cc: Dongwon Kim <dongwon.kim@xxxxxxxxx>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@xxxxxxxxx>
> ---
>  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




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]