diff mbox

virtio-gpu: fix memory leak in resource attach backing

Message ID 1483003721-65360-1-git-send-email-liq3ea@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Li Qiang Dec. 29, 2016, 9:28 a.m. UTC 
In the resource attach backing function, everytime it will
allocate 'res->iov' thus can leading a memory leak. This
patch avoid this.

Signed-off-by: Li Qiang <liq3ea@gmail.com>
---
 hw/display/virtio-gpu.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Marc-André Lureau Dec. 29, 2016, 2:57 p.m. UTC | #1 
Hi

----- Original Message -----
> In the resource attach backing function, everytime it will
> allocate 'res->iov' thus can leading a memory leak. This
> patch avoid this.
> 
> Signed-off-by: Li Qiang <liq3ea@gmail.com>
> ---
>  hw/display/virtio-gpu.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 5f32e1a..045f296 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -705,6 +705,11 @@ virtio_gpu_resource_attach_backing(VirtIOGPU *g,
>          return;
>      }
>  
> +    if (res->iov) {
> +        cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> +        return;
> +    }
> +

I think I would rather return an error in the library for this case.

>      ret = virtio_gpu_create_mapping_iov(&ab, cmd, &res->addrs, &res->iov);
>      if (ret != 0) {
>          cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> --
> 1.8.3.1
> 
>
Li Qiang Dec. 30, 2016, 6:13 a.m. UTC | #2 
2016-12-29 22:57 GMT+08:00 Marc-André Lureau <mlureau@redhat.com>:

> Hi
>
> ----- Original Message -----
> > In the resource attach backing function, everytime it will
> > allocate 'res->iov' thus can leading a memory leak. This
> > patch avoid this.
> >
> > Signed-off-by: Li Qiang <liq3ea@gmail.com>
> > ---
> >  hw/display/virtio-gpu.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> > index 5f32e1a..045f296 100644
> > --- a/hw/display/virtio-gpu.c
> > +++ b/hw/display/virtio-gpu.c
> > @@ -705,6 +705,11 @@ virtio_gpu_resource_attach_backing(VirtIOGPU *g,
> >          return;
> >      }
> >
> > +    if (res->iov) {
> > +        cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> > +        return;
> > +    }
> > +
>
> I think I would rather return an error in the library for this case.
>
>
IIUC Do you mean virglrenderer library? There is no related to this library.

Thanks.


> >      ret = virtio_gpu_create_mapping_iov(&ab, cmd, &res->addrs,
> &res->iov);
> >      if (ret != 0) {
> >          cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> > --
> > 1.8.3.1
> >
> >
>
Gerd Hoffmann Jan. 3, 2017, 9:41 a.m. UTC | #3 
> > +    if (res->iov) {
> > +        cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
> > +        return;
> > +    }
> > +
> 
> I think I would rather return an error in the library for this case.

-ENOLIBRARY ;)

This is the 2d mode code, so virglrenderer isn't involved at all.

Added patch to the queue (and the 3d one too).

thanks,
  Gerd
diff mbox

Patch

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 5f32e1a..045f296 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -705,6 +705,11 @@  virtio_gpu_resource_attach_backing(VirtIOGPU *g,
         return;
     }
 
+    if (res->iov) {
+        cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+        return;
+    }
+
     ret = virtio_gpu_create_mapping_iov(&ab, cmd, &res->addrs, &res->iov);
     if (ret != 0) {
         cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;