Re: [PATCH] vfio-ccw: plug memory leak while getting region info

[Eric Farman]

On 9/28/20 6:17 AM, Cornelia Huck wrote:
> vfio_get_dev_region_info() unconditionally allocates memory
> for a passed-in vfio_region_info structure (and does not re-use
> an already allocated structure). Therefore, we have to free
> the structure we pass to that function in vfio_ccw_get_region()
> for every region we successfully obtained information for.

Ah, yes. This makes sense. A little messy, but hopefully we're done with
new regions for the time being.

> 
> Reported-by: Alex Williamson <alex.williamson@redhat.com>
> Fixes: 8fadea24de4e ("vfio-ccw: support async command subregion")
> Fixes: 46ea3841edaf ("vfio-ccw: Add support for the schib region")
> Fixes: f030532f2ad6 ("vfio-ccw: Add support for the CRW region and IRQ")
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>

Reviewed-by: Eric Farman <farman@linux.ibm.com>

> ---
>  hw/vfio/ccw.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index ff7f36977994..d2755d7fc5ca 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -491,6 +491,7 @@ static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, 
> Error **errp)
>  
>      vcdev->io_region_offset = info->offset;
>      vcdev->io_region = g_malloc0(info->size);
> +    g_free(info);
>  
>      /* check for the optional async command region */
>      ret = vfio_get_dev_region_info(vdev, VFIO_REGION_TYPE_CCW,
> @@ -503,6 +504,7 @@ static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, 
> Error **errp)
>          }
>          vcdev->async_cmd_region_offset = info->offset;
>          vcdev->async_cmd_region = g_malloc0(info->size);
> +        g_free(info);
>      }
>  
>      ret = vfio_get_dev_region_info(vdev, VFIO_REGION_TYPE_CCW,
> @@ -515,6 +517,7 @@ static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, 
> Error **errp)
>          }
>          vcdev->schib_region_offset = info->offset;
>          vcdev->schib_region = g_malloc(info->size);
> +        g_free(info);
>      }
>  
>      ret = vfio_get_dev_region_info(vdev, VFIO_REGION_TYPE_CCW,
> @@ -528,9 +531,9 @@ static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, 
> Error **errp)
>          }
>          vcdev->crw_region_offset = info->offset;
>          vcdev->crw_region = g_malloc(info->size);
> +        g_free(info);
>      }
>  
> -    g_free(info);
>      return;
>  
>  out_err:
>