| Message ID | 1482999086-59795-1-git-send-email-liq3ea@gmail.com |
|---|---|
| State | New |
| Headers | show |
Hi ----- Original Message ----- > If the virgl_renderer_resource_attach_iov function fails the > 'res_iovs' will be leaked. Add check of the return value to > free the 'res_iovs' when failing. > > Signed-off-by: Li Qiang <liq3ea@gmail.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Interestingly, in vrend_renderer_resource_attach_iov(), if the resource has already been attached, it returns 0 too, which will also leak. I guess the fix is it return an error in this case. > --- > hw/display/virtio-gpu-3d.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c > index 23f39de..07b3691 100644 > --- a/hw/display/virtio-gpu-3d.c > +++ b/hw/display/virtio-gpu-3d.c > @@ -291,8 +291,11 @@ static void virgl_resource_attach_backing(VirtIOGPU *g, > return; > } > > - virgl_renderer_resource_attach_iov(att_rb.resource_id, > - res_iovs, att_rb.nr_entries); > + ret = virgl_renderer_resource_attach_iov(att_rb.resource_id, > + res_iovs, att_rb.nr_entries); > + > + if (ret != 0) > + virtio_gpu_cleanup_mapping_iov(res_iovs, att_rb.nr_entries); > } > > static void virgl_resource_detach_backing(VirtIOGPU *g, > -- > 1.8.3.1 > >
2016-12-29 21:56 GMT+08:00 Marc-André Lureau <mlureau@redhat.com>: > Hi > > ----- Original Message ----- > > If the virgl_renderer_resource_attach_iov function fails the > > 'res_iovs' will be leaked. Add check of the return value to > > free the 'res_iovs' when failing. > > > > Signed-off-by: Li Qiang <liq3ea@gmail.com> > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > Interestingly, in vrend_renderer_resource_attach_iov(), if the resource > has already been attached, it returns 0 too, which will also leak. I guess > the fix is it return an error in this case. > > Right, actually I have write a patch for this here: https://cgit.freedesktop.org/virglrenderer/commit/?id=40b0e7813325b08077b6f541b3989edb2d86d837 But the patch just return 0 if the 'iov' exist. > > --- > > hw/display/virtio-gpu-3d.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c > > index 23f39de..07b3691 100644 > > --- a/hw/display/virtio-gpu-3d.c > > +++ b/hw/display/virtio-gpu-3d.c > > @@ -291,8 +291,11 @@ static void virgl_resource_attach_backing(VirtIOGPU > *g, > > return; > > } > > > > - virgl_renderer_resource_attach_iov(att_rb.resource_id, > > - res_iovs, att_rb.nr_entries); > > + ret = virgl_renderer_resource_attach_iov(att_rb.resource_id, > > + res_iovs, > att_rb.nr_entries); > > + > > + if (ret != 0) > > + virtio_gpu_cleanup_mapping_iov(res_iovs, att_rb.nr_entries); > > > > } > > > > static void virgl_resource_detach_backing(VirtIOGPU *g, > > -- > > 1.8.3.1 > > > > >
diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c index 23f39de..07b3691 100644 --- a/hw/display/virtio-gpu-3d.c +++ b/hw/display/virtio-gpu-3d.c @@ -291,8 +291,11 @@ static void virgl_resource_attach_backing(VirtIOGPU *g, return; } - virgl_renderer_resource_attach_iov(att_rb.resource_id, - res_iovs, att_rb.nr_entries); + ret = virgl_renderer_resource_attach_iov(att_rb.resource_id, + res_iovs, att_rb.nr_entries); + + if (ret != 0) + virtio_gpu_cleanup_mapping_iov(res_iovs, att_rb.nr_entries); } static void virgl_resource_detach_backing(VirtIOGPU *g,
If the virgl_renderer_resource_attach_iov function fails the 'res_iovs' will be leaked. Add check of the return value to free the 'res_iovs' when failing. Signed-off-by: Li Qiang <liq3ea@gmail.com> --- hw/display/virtio-gpu-3d.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)