Hi
On Wed, May 8, 2024 at 10:01 PM <dongwon.kim@intel.com> wrote:
>
> From: Dongwon Kim <dongwon.kim@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.lureau@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.lureau@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é <berrange@redhat.com> and
> Marc-André Lureau <marcandre.lureau@redhat.com>)
>
> removed 'dpy_gl' from all helpers' names
> Defined autoptr clean up function for QemuDmaBuf*
> (Daniel P. Berrangé <berrange@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é <philmd@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é <berrange@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é <berrange@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é <berrange@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.lureau@redhat.com>)
>
> --- Use g_autoptr macro to define *dmabuf for auto clean up instead of
> calling qemu_dmabuf_free
> (Marc-André Lureau <marcandre.lureau@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.lureau@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