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

["Duan, Zhenzhong" <zhenzhong.duan@intel.com>]

Hi,

On 6/29/2024 8:15 PM, Marc-André Lureau wrote:
> Hi
>
> On Fri, Jun 28, 2024 at 1:32 PM Zhenzhong Duan 
> <zhenzhong.duan@intel.com> 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 <zhenzhong.duan@intel.com>
>     ---
>      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