[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/4] xc/tmem: Free temporary buffer used during migration.
On 30/04/2014 21:29, Konrad Rzeszutek Wilk wrote: > 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@xxxxxxxxxx> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> > Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> > --- > 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; > + } Superfluous braces. Otherwise, still fine. ~Andrew > 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 */ _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |