Hi

On Wed, May 8, 2024 at 10:01 PM <dongwon....@intel.com> wrote:
>
> From: Dongwon Kim <dongwon....@intel.com>
>
> This series introduces privacy enhancements to the QemuDmaBuf struct
> and its contained data to bolster security. it accomplishes this by
> introducing of helper functions for allocating, deallocating, and
> accessing individual fields within the struct and replacing all direct
> references to individual fields in the struct with methods using helpers
> throughout the codebase.
>
> This change was made based on a suggestion from Marc-André Lureau
> <marcandre.lur...@redhat.com>
>
> (Resumitting same patch series with this new cover-leter)
>
> v6: fixed some typos in patch -
>     ui/console: Introduce dpy_gl_qemu_dmabuf_get_..() helpers)
>
> v7: included minor fix (ui/gtk: Check if fence_fd is equal to or greater than 
> 0)
>     (Marc-André Lureau <marcandre.lur...@redhat.com>)
>
>     migrated all helpers and QemuDmaBuf struct into dmabuf.c and their 
> prototypes
>     to dmabuf.h for better encapsulation (ui/dmabuf: New dmabuf.c and 
> dmabuf.h..)
>     (Daniel P. Berrangé <berra...@redhat.com> and
>      Marc-André Lureau <marcandre.lur...@redhat.com>)
>
>     removed 'dpy_gl' from all helpers' names
>     Defined autoptr clean up function for QemuDmaBuf*
>     (Daniel P. Berrangé <berra...@redhat.com>)
>
>     Minor corrections
>
> v8: Introduce new dmabuf.c and dmabuf.h and all helper functions in the second
>     patch in the series (ui/console: new dmabuf.h and dmabuf.c for 
> QemuDma....)
>     (Philippe Mathieu-Daudé <phi...@linaro.org>)
>
> v9: set dmabuf->allow_fences true when it is created in virtio-gpu-udmabuf
>
>     removed unnecessary spaces were added in the patch,
>     'ui/console: Use qemu_dmabuf_new() a...'
>
> v10: Change the license type for both dmabuf.h and dmabuf.c from MIT to
>      GPL to be in line with QEMU's default license
>      (Daniel P. Berrangé <berra...@redhat.com>)
>
> v11: New helpers added - qemu_dmabuf_dup_fd, qemu_dmabuf_close for duplicating
>      and closing dmabuf->fd. And use them in places where applicable.
>      (Daniel P. Berrangé <berra...@redhat.com>)
>
>      qemu_dmabuf_free helper now close dmabuf->fd before freeing the struct to
>      prevent any potential leakage (This eliminates the need for
>      qemu_dmabuf_close in several places as qemu_dmabuf_close is done anyway.)
>      (Daniel P. Berrangé <berra...@redhat.com>)
>
> v12: --- qemu_dmabuf_free does not include qemu_dmabuf_close as there are 
> cases
>          where fd still needs to be used even after QemuDmaBuf struct is
>          destroyed (virtio-gpu: res->dmabuf_fd)
>
>      --- 'dmabuf' is now allocated space so it should be freed at the end of
>          dbus_scanout_texture
>
> v13: --- Immediately free dmabuf after it is released to prevent possible
>          leaking of the ptr
>          (Marc-André Lureau <marcandre.lur...@redhat.com>)
>
>      --- Use g_autoptr macro to define *dmabuf for auto clean up instead of
>          calling qemu_dmabuf_free
>          (Marc-André Lureau <marcandre.lur...@redhat.com>)
>
> v14: In ui/console: Use qemu_dmabuf_new() and free() helpers instead
>
>      --- (vhost-user-gpu) Change qemu_dmabuf_free back to g_clear_pointer
>          as it was done because of some misunderstanding (v13).
>
>      --- (vhost-user-gpu) g->dmabuf[m->scanout_id] needs to be set to NULL
>          to prevent freed dmabuf to be accessed again in case if(fd==-1)break;
>          happens (before new dmabuf is allocated). Otherwise, it would cause
>          invalid memory access when the same function is executed. Also NULL
>          check should be done before qemu_dmabuf_close (it asserts 
> dmabuf!=NULL.).
>          (Marc-André Lureau <marcandre.lur...@redhat.com>)
>

thanks, queued

> Dongwon Kim (6):
>   ui/gtk: Check if fence_fd is equal to or greater than 0
>   ui/console: new dmabuf.h and dmabuf.c for QemuDmaBuf struct and
>     helpers
>   ui/console: Use qemu_dmabuf_get_..() helpers instead
>   ui/console: Use qemu_dmabuf_set_..() helpers instead
>   ui/console: Use qemu_dmabuf_new() and free() helpers instead
>   ui/console: move QemuDmaBuf struct def to dmabuf.c
>
>  include/hw/vfio/vfio-common.h   |   2 +-
>  include/hw/virtio/virtio-gpu.h  |   4 +-
>  include/ui/console.h            |  20 +--
>  include/ui/dmabuf.h             |  49 +++++++
>  hw/display/vhost-user-gpu.c     |  32 +++--
>  hw/display/virtio-gpu-udmabuf.c |  27 ++--
>  hw/vfio/display.c               |  32 ++---
>  ui/console.c                    |   4 +-
>  ui/dbus-console.c               |   9 +-
>  ui/dbus-listener.c              |  71 +++++-----
>  ui/dmabuf.c                     | 229 ++++++++++++++++++++++++++++++++
>  ui/egl-headless.c               |  23 +++-
>  ui/egl-helpers.c                |  59 ++++----
>  ui/gtk-egl.c                    |  52 +++++---
>  ui/gtk-gl-area.c                |  41 ++++--
>  ui/gtk.c                        |  12 +-
>  ui/spice-display.c              |  50 ++++---
>  ui/meson.build                  |   1 +
>  18 files changed, 524 insertions(+), 193 deletions(-)
>  create mode 100644 include/ui/dmabuf.h
>  create mode 100644 ui/dmabuf.c
>
> --
> 2.34.1
>
>


-- 
Marc-André Lureau

Reply via email to