Patchwork [RFC,2/3] xen_disk: fix memory leak

login
register
mail settings
Submitter Roger Pau Monne
Date Dec. 31, 2012, 12:16 p.m.
Message ID <1356956174-23548-3-git-send-email-roger.pau@citrix.com>
Download mbox | patch
Permalink /patch/208846/
State New
Headers show

Comments

Roger Pau Monne - Dec. 31, 2012, 12:16 p.m.
On ioreq_release the full ioreq was memset to 0, loosing all the data
and memory allocations inside the QEMUIOVector, which leads to a
memory leak. Create a new function to specifically reset ioreq.

Reported-by: Maik Wessler <maik.wessler@yahoo.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: xen-devel@lists.xen.org
Cc: Stefano Stabellini <Stefano.Stabellini@eu.citrix.com>
Cc: Anthony PERARD <anthony.perard@citrix.com>
---
 hw/xen_disk.c |   28 ++++++++++++++++++++++++++--
 1 files changed, 26 insertions(+), 2 deletions(-)
Stefano Stabellini - Jan. 4, 2013, 3:06 p.m.
On Mon, 31 Dec 2012, Roger Pau Monne wrote:
> On ioreq_release the full ioreq was memset to 0, loosing all the data
> and memory allocations inside the QEMUIOVector, which leads to a
> memory leak. Create a new function to specifically reset ioreq.
> 
> Reported-by: Maik Wessler <maik.wessler@yahoo.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: xen-devel@lists.xen.org
> Cc: Stefano Stabellini <Stefano.Stabellini@eu.citrix.com>
> Cc: Anthony PERARD <anthony.perard@citrix.com>
>

Nice catch!

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

> ---
>  hw/xen_disk.c |   28 ++++++++++++++++++++++++++--
>  1 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/xen_disk.c b/hw/xen_disk.c
> index a159ee5..1eb485a 100644
> --- a/hw/xen_disk.c
> +++ b/hw/xen_disk.c
> @@ -113,6 +113,31 @@ struct XenBlkDev {
>  
>  /* ------------------------------------------------------------- */
>  
> +static void ioreq_reset(struct ioreq *ioreq)
> +{
> +    memset(&ioreq->req, 0, sizeof(ioreq->req));
> +    ioreq->status = 0;
> +    ioreq->start = 0;
> +    ioreq->presync = 0;
> +    ioreq->postsync = 0;
> +    ioreq->mapped = 0;
> +
> +    memset(ioreq->domids, 0, sizeof(ioreq->domids));
> +    memset(ioreq->refs, 0, sizeof(ioreq->refs));
> +    ioreq->prot = 0;
> +    memset(ioreq->page, 0, sizeof(ioreq->page));
> +    ioreq->pages = NULL;
> +
> +    ioreq->aio_inflight = 0;
> +    ioreq->aio_errors = 0;
> +
> +    ioreq->blkdev = NULL;
> +    memset(&ioreq->list, 0, sizeof(ioreq->list));
> +    memset(&ioreq->acct, 0, sizeof(ioreq->acct));
> +
> +    qemu_iovec_reset(&ioreq->v);
> +}
> +
>  static struct ioreq *ioreq_start(struct XenBlkDev *blkdev)
>  {
>      struct ioreq *ioreq = NULL;
> @@ -130,7 +155,6 @@ static struct ioreq *ioreq_start(struct XenBlkDev *blkdev)
>          /* get one from freelist */
>          ioreq = QLIST_FIRST(&blkdev->freelist);
>          QLIST_REMOVE(ioreq, list);
> -        qemu_iovec_reset(&ioreq->v);
>      }
>      QLIST_INSERT_HEAD(&blkdev->inflight, ioreq, list);
>      blkdev->requests_inflight++;
> @@ -154,7 +178,7 @@ static void ioreq_release(struct ioreq *ioreq, bool finish)
>      struct XenBlkDev *blkdev = ioreq->blkdev;
>  
>      QLIST_REMOVE(ioreq, list);
> -    memset(ioreq, 0, sizeof(*ioreq));
> +    ioreq_reset(ioreq);
>      ioreq->blkdev = blkdev;
>      QLIST_INSERT_HEAD(&blkdev->freelist, ioreq, list);
>      if (finish) {
> -- 
> 1.7.7.5 (Apple Git-26)
> 
>

Patch

diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index a159ee5..1eb485a 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -113,6 +113,31 @@  struct XenBlkDev {
 
 /* ------------------------------------------------------------- */
 
+static void ioreq_reset(struct ioreq *ioreq)
+{
+    memset(&ioreq->req, 0, sizeof(ioreq->req));
+    ioreq->status = 0;
+    ioreq->start = 0;
+    ioreq->presync = 0;
+    ioreq->postsync = 0;
+    ioreq->mapped = 0;
+
+    memset(ioreq->domids, 0, sizeof(ioreq->domids));
+    memset(ioreq->refs, 0, sizeof(ioreq->refs));
+    ioreq->prot = 0;
+    memset(ioreq->page, 0, sizeof(ioreq->page));
+    ioreq->pages = NULL;
+
+    ioreq->aio_inflight = 0;
+    ioreq->aio_errors = 0;
+
+    ioreq->blkdev = NULL;
+    memset(&ioreq->list, 0, sizeof(ioreq->list));
+    memset(&ioreq->acct, 0, sizeof(ioreq->acct));
+
+    qemu_iovec_reset(&ioreq->v);
+}
+
 static struct ioreq *ioreq_start(struct XenBlkDev *blkdev)
 {
     struct ioreq *ioreq = NULL;
@@ -130,7 +155,6 @@  static struct ioreq *ioreq_start(struct XenBlkDev *blkdev)
         /* get one from freelist */
         ioreq = QLIST_FIRST(&blkdev->freelist);
         QLIST_REMOVE(ioreq, list);
-        qemu_iovec_reset(&ioreq->v);
     }
     QLIST_INSERT_HEAD(&blkdev->inflight, ioreq, list);
     blkdev->requests_inflight++;
@@ -154,7 +178,7 @@  static void ioreq_release(struct ioreq *ioreq, bool finish)
     struct XenBlkDev *blkdev = ioreq->blkdev;
 
     QLIST_REMOVE(ioreq, list);
-    memset(ioreq, 0, sizeof(*ioreq));
+    ioreq_reset(ioreq);
     ioreq->blkdev = blkdev;
     QLIST_INSERT_HEAD(&blkdev->freelist, ioreq, list);
     if (finish) {