Re: [PATCH 1/2] vfio/display: Fix potential memleak of edid info

2024-06-30 Thread Duan, Zhenzhong

Hi,

On 6/29/2024 8:15 PM, Marc-André Lureau wrote:

Hi

On Fri, Jun 28, 2024 at 1:32 PM Zhenzhong Duan 
 wrote:


EDID related device region info is leaked in three paths:
1. In vfio_get_dev_region_info(), when edid info isn't find, the last
device region info is leaked.
2. In vfio_display_edid_init() error path, edid info is leaked.
3. In VFIODisplay destroying path, edid info is leaked.

Fixes: 08479114b0de ("vfio/display: add edid support.")
Signed-off-by: Zhenzhong Duan 
---
 hw/vfio/display.c | 2 ++
 hw/vfio/helpers.c | 1 +
 2 files changed, 3 insertions(+)

diff --git a/hw/vfio/display.c b/hw/vfio/display.c
index 661e921616..5926bd6628 100644
--- a/hw/vfio/display.c
+++ b/hw/vfio/display.c
@@ -171,6 +171,7 @@ static void
vfio_display_edid_init(VFIOPCIDevice *vdev)

 err:
     trace_vfio_display_edid_write_error();
+    g_free(dpy->edid_info);


It would be better to set it to NULL.

Will do.


     g_free(dpy->edid_regs);
     dpy->edid_regs = NULL;
     return;
@@ -182,6 +183,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);
diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c
index b14edd46ed..3dd32b26a4 100644
--- a/hw/vfio/helpers.c
+++ b/hw/vfio/helpers.c
@@ -586,6 +586,7 @@ int vfio_get_dev_region_info(VFIODevice
*vbasedev, uint32_t type,
         g_free(*info);
     }

+    g_free(*info);


This seems incorrect, it is freed at the end of the loop above if it 
didn't retun.


Good catch! Will remove it.

Thanks

Zhenzhong



     *info = NULL;
     return -ENODEV;
 }
-- 
2.34.1





--
Marc-André Lureau




Re: [PATCH 1/2] vfio/display: Fix potential memleak of edid info

2024-06-29 Thread Marc-André Lureau
Hi

On Fri, Jun 28, 2024 at 1:32 PM Zhenzhong Duan 
wrote:

> EDID related device region info is leaked in three paths:
> 1. In vfio_get_dev_region_info(), when edid info isn't find, the last
> device region info is leaked.
> 2. In vfio_display_edid_init() error path, edid info is leaked.
> 3. In VFIODisplay destroying path, edid info is leaked.
>
> Fixes: 08479114b0de ("vfio/display: add edid support.")
> Signed-off-by: Zhenzhong Duan 
> ---
>  hw/vfio/display.c | 2 ++
>  hw/vfio/helpers.c | 1 +
>  2 files changed, 3 insertions(+)
>
> diff --git a/hw/vfio/display.c b/hw/vfio/display.c
> index 661e921616..5926bd6628 100644
> --- a/hw/vfio/display.c
> +++ b/hw/vfio/display.c
> @@ -171,6 +171,7 @@ static void vfio_display_edid_init(VFIOPCIDevice *vdev)
>
>  err:
>  trace_vfio_display_edid_write_error();
> +g_free(dpy->edid_info);
>

It would be better to set it to NULL.


>  g_free(dpy->edid_regs);
>  dpy->edid_regs = NULL;
>  return;
> @@ -182,6 +183,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);
> diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c
> index b14edd46ed..3dd32b26a4 100644
> --- a/hw/vfio/helpers.c
> +++ b/hw/vfio/helpers.c
> @@ -586,6 +586,7 @@ int vfio_get_dev_region_info(VFIODevice *vbasedev,
> uint32_t type,
>  g_free(*info);
>  }
>
> +g_free(*info);
>

This seems incorrect, it is freed at the end of the loop above if it didn't
retun.


>  *info = NULL;
>  return -ENODEV;
>  }
> --
> 2.34.1
>
>
>

-- 
Marc-André Lureau


[PATCH 1/2] vfio/display: Fix potential memleak of edid info

2024-06-28 Thread Zhenzhong Duan
EDID related device region info is leaked in three paths:
1. In vfio_get_dev_region_info(), when edid info isn't find, the last
device region info is leaked.
2. In vfio_display_edid_init() error path, edid info is leaked.
3. In VFIODisplay destroying path, edid info is leaked.

Fixes: 08479114b0de ("vfio/display: add edid support.")
Signed-off-by: Zhenzhong Duan 
---
 hw/vfio/display.c | 2 ++
 hw/vfio/helpers.c | 1 +
 2 files changed, 3 insertions(+)

diff --git a/hw/vfio/display.c b/hw/vfio/display.c
index 661e921616..5926bd6628 100644
--- a/hw/vfio/display.c
+++ b/hw/vfio/display.c
@@ -171,6 +171,7 @@ 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_regs = NULL;
 return;
@@ -182,6 +183,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);
diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c
index b14edd46ed..3dd32b26a4 100644
--- a/hw/vfio/helpers.c
+++ b/hw/vfio/helpers.c
@@ -586,6 +586,7 @@ int vfio_get_dev_region_info(VFIODevice *vbasedev, uint32_t 
type,
 g_free(*info);
 }
 
+g_free(*info);
 *info = NULL;
 return -ENODEV;
 }
-- 
2.34.1