[Xen-devel] [PATCH v2 1/3] xc/tmem: Free temporary buffer used during migration

[Bob Liu <lliubbo@xxxxxxxxx>]

From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>

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.

v2:
* Remove superfluous braces. (Andrew)

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Signed-off-by: Bob Liu <bob.liu@xxxxxxxxxx>
Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
 tools/libxc/xc_tmem.c |   44 ++++++++++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 18 deletions(-)

diff --git a/tools/libxc/xc_tmem.c b/tools/libxc/xc_tmem.c
index 3261e10..6832bbe 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,13 +290,13 @@ 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 {
@@ -304,7 +304,7 @@ int xc_tmem_save(xc_interface *xch,
                     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;
+                        goto failed;
                     break;
                 }
             }
@@ -315,9 +315,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 +390,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 +428,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 +449,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 +457,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 +478,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.7.10.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel