From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: [PATCH 1/4] xc/tmem: Free temporary buffer used during migration. Date: Wed, 30 Apr 2014 16:29:13 -0400 Message-ID: <1398889756-16352-2-git-send-email-konrad.wilk@oracle.com> References: <1398889756-16352-1-git-send-email-konrad.wilk@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Wfb8A-0001ds-BV for xen-devel@lists.xenproject.org; Wed, 30 Apr 2014 20:29:26 +0000 In-Reply-To: <1398889756-16352-1-git-send-email-konrad.wilk@oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel@lists.xenproject.org, andrew.cooper3@citrix.com, jbeulich@suse.com, keir@xen.org Cc: Konrad Rzeszutek Wilk List-Id: xen-devel@lists.xenproject.org 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 Signed-off-by: Konrad Rzeszutek Wilk Reviewed-by: Andrew Cooper --- 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