| Message ID | 20200928101701.13540-1-cohuck@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | vfio-ccw: plug memory leak while getting region info | expand |
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: >
On 9/28/20 12:17 PM, 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. > > 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> > --- > hw/vfio/ccw.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On Mon, 28 Sep 2020 12:17:01 +0200 Cornelia Huck <cohuck@redhat.com> 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. > > 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> > --- > hw/vfio/ccw.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) Queued to s390-next.
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:
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. 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> --- hw/vfio/ccw.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)