* [PATCH v2 1/2] vfio/display: Fix potential memleak of edid info
2024-07-01 1:48 [PATCH v2 0/2] Misc fixes on vfio display Zhenzhong Duan
@ 2024-07-01 1:48 ` Zhenzhong Duan
2024-07-02 15:25 ` Cédric Le Goater
2024-07-02 15:31 ` Marc-André Lureau
2024-07-01 1:48 ` [PATCH v2 2/2] vfio/display: Fix vfio_display_edid_init() error path Zhenzhong Duan
2024-07-02 16:05 ` [PATCH v2 0/2] Misc fixes on vfio display Cédric Le Goater
2 siblings, 2 replies; 6+ messages in thread
From: Zhenzhong Duan @ 2024-07-01 1:48 UTC (permalink / raw)
To: qemu-devel
Cc: alex.williamson, clg, marcandre.lureau, kraxel, chao.p.peng,
Zhenzhong Duan
EDID related device region info is leaked in vfio_display_edid_init()
error path and VFIODisplay destroying path.
Fixes: 08479114b0de ("vfio/display: add edid support.")
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
hw/vfio/display.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/vfio/display.c b/hw/vfio/display.c
index 661e921616..9c57fd3888 100644
--- a/hw/vfio/display.c
+++ b/hw/vfio/display.c
@@ -171,7 +171,9 @@ static void vfio_display_edid_init(VFIOPCIDevice *vdev)
err:
trace_vfio_display_edid_write_error();
+ g_free(dpy->edid_info);
g_free(dpy->edid_regs);
+ dpy->edid_info = NULL;
dpy->edid_regs = NULL;
return;
}
@@ -182,6 +184,7 @@ static void vfio_display_edid_exit(VFIODisplay *dpy)
return;
}
+ g_free(dpy->edid_info);
g_free(dpy->edid_regs);
g_free(dpy->edid_blob);
timer_free(dpy->edid_link_timer);
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/2] vfio/display: Fix vfio_display_edid_init() error path
2024-07-01 1:48 [PATCH v2 0/2] Misc fixes on vfio display Zhenzhong Duan
2024-07-01 1:48 ` [PATCH v2 1/2] vfio/display: Fix potential memleak of edid info Zhenzhong Duan
@ 2024-07-01 1:48 ` Zhenzhong Duan
2024-07-02 16:05 ` [PATCH v2 0/2] Misc fixes on vfio display Cédric Le Goater
2 siblings, 0 replies; 6+ messages in thread
From: Zhenzhong Duan @ 2024-07-01 1:48 UTC (permalink / raw)
To: qemu-devel
Cc: alex.williamson, clg, marcandre.lureau, kraxel, chao.p.peng,
Zhenzhong Duan
vfio_display_edid_init() can fail for many reasons and return silently.
It would be good to report the error.
Old mdev driver may not support vfio edid region and we allow to go
through in this case.
vfio_display_edid_update() isn't changed because it can be called at
runtime when UI changes (i.e. window resize).
Fixes: 08479114b0de ("vfio/display: add edid support.")
Suggested-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/vfio/display.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/hw/vfio/display.c b/hw/vfio/display.c
index 9c57fd3888..ea87830fe0 100644
--- a/hw/vfio/display.c
+++ b/hw/vfio/display.c
@@ -124,7 +124,7 @@ static void vfio_display_edid_ui_info(void *opaque, uint32_t idx,
}
}
-static void vfio_display_edid_init(VFIOPCIDevice *vdev)
+static bool vfio_display_edid_init(VFIOPCIDevice *vdev, Error **errp)
{
VFIODisplay *dpy = vdev->dpy;
int fd = vdev->vbasedev.fd;
@@ -135,7 +135,8 @@ static void vfio_display_edid_init(VFIOPCIDevice *vdev)
VFIO_REGION_SUBTYPE_GFX_EDID,
&dpy->edid_info);
if (ret) {
- return;
+ /* Failed to get GFX edid info, allow to go through without edid. */
+ return true;
}
trace_vfio_display_edid_available();
@@ -167,15 +168,16 @@ static void vfio_display_edid_init(VFIOPCIDevice *vdev)
vfio_display_edid_link_up, vdev);
vfio_display_edid_update(vdev, true, 0, 0);
- return;
+ return true;
err:
+ error_setg(errp, "vfio: failed to read GFX edid field");
trace_vfio_display_edid_write_error();
g_free(dpy->edid_info);
g_free(dpy->edid_regs);
dpy->edid_info = NULL;
dpy->edid_regs = NULL;
- return;
+ return false;
}
static void vfio_display_edid_exit(VFIODisplay *dpy)
@@ -368,8 +370,7 @@ static bool vfio_display_dmabuf_init(VFIOPCIDevice *vdev, Error **errp)
return false;
}
}
- vfio_display_edid_init(vdev);
- return true;
+ return vfio_display_edid_init(vdev, errp);
}
static void vfio_display_dmabuf_exit(VFIODisplay *dpy)
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 0/2] Misc fixes on vfio display
2024-07-01 1:48 [PATCH v2 0/2] Misc fixes on vfio display Zhenzhong Duan
2024-07-01 1:48 ` [PATCH v2 1/2] vfio/display: Fix potential memleak of edid info Zhenzhong Duan
2024-07-01 1:48 ` [PATCH v2 2/2] vfio/display: Fix vfio_display_edid_init() error path Zhenzhong Duan
@ 2024-07-02 16:05 ` Cédric Le Goater
2 siblings, 0 replies; 6+ messages in thread
From: Cédric Le Goater @ 2024-07-02 16:05 UTC (permalink / raw)
To: Zhenzhong Duan, qemu-devel
Cc: alex.williamson, marcandre.lureau, kraxel, chao.p.peng
On 7/1/24 3:48 AM, Zhenzhong Duan wrote:
> Hi,
>
> This is trying to address an issue Cédric found.
> See https://www.mail-archive.com/qemu-devel@nongnu.org/msg1043142.html
> While looking into it, also found a potential memory leak.
>
> I'm sorry that I didn't find how to test this fix, because it looks
> a GFX card is needed. Any idea on how to test or help test are quite
> appreciated.
>
> Thanks
> Zhenzhong
>
> v2:
> - set dpy->edid_info to NULL in vfio_display_edid_init() err path (Marc-André)
> - remove a wrongly added g_free(*info) in vfio_get_dev_region_info() (Marc-André)
> - add R-B on patch2
>
>
> Zhenzhong Duan (2):
> vfio/display: Fix potential memleak of edid info
> vfio/display: Fix vfio_display_edid_init() error path
>
> hw/vfio/display.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
Applied to vfio-next.
Thanks,
C.
^ permalink raw reply [flat|nested] 6+ messages in thread