xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: xen-devel@lists.xenproject.org, andrew.cooper3@citrix.com,
	jbeulich@suse.com, keir@xen.org
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Subject: [PATCH 1/4] xc/tmem: Free temporary buffer used during migration.
Date: Wed, 30 Apr 2014 16:29:13 -0400	[thread overview]
Message-ID: <1398889756-16352-2-git-send-email-konrad.wilk@oracle.com> (raw)
In-Reply-To: <1398889756-16352-1-git-send-email-konrad.wilk@oracle.com>

CID 1090388.

Within the loop reading the pool_id we set the buf:

 if ( (buf = realloc(buf,bufsize)) == NULL )

and then continue on using it without freeing. Worst yet
there are multiple 'if (..) return -1' which do not free
the buffer.

As such insert a 'fail' goto label to free the buffer
and also add on the OK path a mechanism to free the buffer.

Replace all of the 'return -1' with a jump to the failed
label.

CC: Bob Liu <bob.liu@oracle.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 tools/libxc/xc_tmem.c | 47 ++++++++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/tools/libxc/xc_tmem.c b/tools/libxc/xc_tmem.c
index 3261e10..12fa105 100644
--- a/tools/libxc/xc_tmem.c
+++ b/tools/libxc/xc_tmem.c
@@ -215,6 +215,7 @@ int xc_tmem_save(xc_interface *xch,
     uint32_t pool_id;
     uint32_t minusone = -1;
     struct tmem_handle *h;
+    char *buf = NULL;
 
     if ( xc_tmem_control(xch,0,TMEMC_SAVE_BEGIN,dom,live,0,0,NULL) <= 0 )
         return 0;
@@ -249,7 +250,6 @@ int xc_tmem_save(xc_interface *xch,
         uint64_t uuid[2];
         uint32_t n_pages;
         uint32_t pagesize;
-        char *buf = NULL;
         int bufsize = 0;
         int checksum = 0;
 
@@ -263,13 +263,13 @@ int xc_tmem_save(xc_interface *xch,
                 n_pages = 0;
             (void)xc_tmem_control(xch,i,TMEMC_SAVE_GET_POOL_UUID,dom,sizeof(uuid),0,0,&uuid);
             if ( write_exact(io_fd, &pool_id, sizeof(pool_id)) )
-                return -1;
+                goto failed;
             if ( write_exact(io_fd, &flags, sizeof(flags)) )
-                return -1;
+                goto failed;
             if ( write_exact(io_fd, &n_pages, sizeof(n_pages)) )
-                return -1;
+                goto failed;
             if ( write_exact(io_fd, &uuid, sizeof(uuid)) )
-                return -1;
+                goto failed;
             if ( n_pages == 0 )
                 continue;
 
@@ -279,7 +279,7 @@ int xc_tmem_save(xc_interface *xch,
             {
                 bufsize = pagesize + sizeof(struct tmem_handle);
                 if ( (buf = realloc(buf,bufsize)) == NULL )
-                    return -1;
+                    goto failed;
             }
             for ( j = n_pages; j > 0; j-- )
             {
@@ -290,21 +290,22 @@ int xc_tmem_save(xc_interface *xch,
                 {
                     h = (struct tmem_handle *)buf;
                     if ( write_exact(io_fd, &h->oid, sizeof(h->oid)) )
-                        return -1;
+                        goto failed;
                     if ( write_exact(io_fd, &h->index, sizeof(h->index)) )
-                        return -1;
+                        goto failed;
                     h++;
                     checksum += *(char *)h;
                     if ( write_exact(io_fd, h, pagesize) )
-                        return -1;
+                        goto failed;
                 } else if ( ret == 0 ) {
                     continue;
                 } else {
                     /* page list terminator */
                     h = (struct tmem_handle *)buf;
                     h->oid[0] = h->oid[1] = h->oid[2] = -1L;
-                    if ( write_exact(io_fd, &h->oid, sizeof(h->oid)) )
-                        return -1;
+                    if ( write_exact(io_fd, &h->oid, sizeof(h->oid)) ) {
+                        goto failed;
+                    }
                     break;
                 }
             }
@@ -315,9 +316,13 @@ int xc_tmem_save(xc_interface *xch,
     /* pool list terminator */
     minusone = -1;
     if ( write_exact(io_fd, &minusone, sizeof(minusone)) )
-        return -1;
+        goto failed;
 
+    free(buf);
     return 1;
+failed:
+    free(buf);
+    return -1;
 }
 
 /* only called for live migration */
@@ -386,6 +391,7 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
     uint32_t minusone;
     uint32_t weight, cap, flags;
     int checksum = 0;
+    char *buf = NULL;
 
     save_version = xc_tmem_control(xch,0,TMEMC_SAVE_GET_VERSION,dom,0,0,0,NULL);
     if ( save_version == -1 )
@@ -423,7 +429,6 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
     {
         uint64_t uuid[2];
         uint32_t n_pages;
-        char *buf = NULL;
         int bufsize = 0, pagesize;
         int j;
 
@@ -445,7 +450,7 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
         {
             bufsize = pagesize;
             if ( (buf = realloc(buf,bufsize)) == NULL )
-                return -1;
+                goto failed;
         }
         for ( j = n_pages; j > 0; j-- )
         {
@@ -453,20 +458,20 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
             uint32_t index;
             int rc;
             if ( read_exact(io_fd, &oid, sizeof(oid)) )
-                return -1;
+                goto failed;
             if ( oid.oid[0] == -1L && oid.oid[1] == -1L && oid.oid[2] == -1L )
                 break;
             if ( read_exact(io_fd, &index, sizeof(index)) )
-                return -1;
+                goto failed;
             if ( read_exact(io_fd, buf, pagesize) )
-                return -1;
+                goto failed;
             checksum += *buf;
             if ( (rc = xc_tmem_control_oid(xch, pool_id,
                                            TMEMC_RESTORE_PUT_PAGE, dom,
                                            bufsize, index, oid, buf)) <= 0 )
             {
                 DPRINTF("xc_tmem_restore: putting page failed, rc=%d\n",rc);
-                return -1;
+                goto failed;
             }
         }
         if ( n_pages )
@@ -474,9 +479,13 @@ int xc_tmem_restore(xc_interface *xch, int dom, int io_fd)
                     n_pages-j,dom,pool_id,checksum);
     }
     if ( pool_id != -1 )
-        return -1;
+        goto failed;
 
+    free(buf);
     return 0;
+failed:
+    free(buf);
+    return -1;
 }
 
 /* only called for live migration, must be called after suspend */
-- 
1.8.5.3

  reply	other threads:[~2014-04-30 20:29 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-30 20:29 [PATCH] tmem fixes for Xen 4.5 - coverity inspired Konrad Rzeszutek Wilk
2014-04-30 20:29 ` Konrad Rzeszutek Wilk [this message]
2014-04-30 20:43   ` [PATCH 1/4] xc/tmem: Free temporary buffer used during migration Andrew Cooper
2014-04-30 20:29 ` [PATCH 2/4] xc/tmem: Unchecked return value (v2) Konrad Rzeszutek Wilk
2014-04-30 20:46   ` Andrew Cooper
2014-04-30 20:29 ` [PATCH 3/4] tmem: drop unnecessary lock in tmem_relinquish_pages() Konrad Rzeszutek Wilk
2014-04-30 20:29 ` [PATCH 4/4] tmem: fix Out-of-bounds read reported by Coverity Konrad Rzeszutek Wilk
2014-04-30 21:04   ` Andrew Cooper
2014-05-04  8:10     ` Bob Liu
2014-05-05 11:08       ` Andrew Cooper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1398889756-16352-2-git-send-email-konrad.wilk@oracle.com \
    --to=konrad.wilk@oracle.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=keir@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).