Fix double-free in xc_interface_close(), xenpaging_teardown() releases the *xch already. Remove second xc_interface_close() call. Fix DPRINTF/ERROR usage, both macros reference a xch variable in local scope. If xc_interface_open fails for some reason, and after xc_interface_close, both can not be used anymore. Use standard printf for this case. Instead of passing xch around, use the handle from *paging. In the updated functions, use the local xch variable. Signed-off-by: Olaf Hering <olaf@aepfle.de> --- tools/xenpaging/policy.h | 3 - tools/xenpaging/policy_default.c | 4 +- tools/xenpaging/xenpaging.c | 65 +++++++++++++++++++-------------------- 3 files changed, 36 insertions(+), 36 deletions(-) --- xen-unstable.hg-4.1.22447.orig/tools/xenpaging/policy.h +++ xen-unstable.hg-4.1.22447/tools/xenpaging/policy.h @@ -29,8 +29,7 @@ int policy_init(xenpaging_t *paging); -int policy_choose_victim(xc_interface *xch, - xenpaging_t *paging, domid_t domain_id, +int policy_choose_victim(xenpaging_t *paging, domid_t domain_id, xenpaging_victim_t *victim); void policy_notify_paged_out(domid_t domain_id, unsigned long gfn); void policy_notify_paged_in(domid_t domain_id, unsigned long gfn); --- xen-unstable.hg-4.1.22447.orig/tools/xenpaging/policy_default.c +++ xen-unstable.hg-4.1.22447/tools/xenpaging/policy_default.c @@ -67,10 +67,10 @@ int policy_init(xenpaging_t *paging) return rc; } -int policy_choose_victim(xc_interface *xch, - xenpaging_t *paging, domid_t domain_id, +int policy_choose_victim(xenpaging_t *paging, domid_t domain_id, xenpaging_victim_t *victim) { + struct xc_interface *xch = paging->xc_handle; unsigned long wrap = current_gfn; ASSERT(victim != NULL); --- xen-unstable.hg-4.1.22447.orig/tools/xenpaging/xenpaging.c +++ xen-unstable.hg-4.1.22447/tools/xenpaging/xenpaging.c @@ -79,7 +79,7 @@ static void *init_page(void) return NULL; } -xenpaging_t *xenpaging_init(xc_interface **xch_r, domid_t domain_id) +xenpaging_t *xenpaging_init(domid_t domain_id) { xenpaging_t *paging; xc_interface *xch; @@ -90,7 +90,6 @@ xenpaging_t *xenpaging_init(xc_interface goto err_iface; DPRINTF("xenpaging init\n"); - *xch_r = xch; /* Allocate memory */ paging = malloc(sizeof(xenpaging_t)); @@ -128,7 +127,7 @@ xenpaging_t *xenpaging_init(xc_interface mem_event_ring_lock_init(&paging->mem_event); /* Initialise Xen */ - rc = xc_mem_event_enable(paging->xc_handle, paging->mem_event.domain_id, + rc = xc_mem_event_enable(xch, paging->mem_event.domain_id, paging->mem_event.shared_page, paging->mem_event.ring_page); if ( rc != 0 ) @@ -175,7 +174,7 @@ xenpaging_t *xenpaging_init(xc_interface goto err; } - rc = xc_get_platform_info(paging->xc_handle, domain_id, + rc = xc_get_platform_info(xch, domain_id, paging->platform_info); if ( rc != 1 ) { @@ -191,7 +190,7 @@ xenpaging_t *xenpaging_init(xc_interface goto err; } - rc = xc_domain_getinfolist(paging->xc_handle, domain_id, 1, + rc = xc_domain_getinfolist(xch, domain_id, 1, paging->domain_info); if ( rc != 1 ) { @@ -224,6 +223,7 @@ xenpaging_t *xenpaging_init(xc_interface err: if ( paging ) { + xc_interface_close(xch); if ( paging->mem_event.shared_page ) { munlock(paging->mem_event.shared_page, PAGE_SIZE); @@ -246,15 +246,18 @@ xenpaging_t *xenpaging_init(xc_interface return NULL; } -int xenpaging_teardown(xc_interface *xch, xenpaging_t *paging) +int xenpaging_teardown(xenpaging_t *paging) { int rc; + struct xc_interface *xch; if ( paging == NULL ) return 0; + xch = paging->xc_handle; + paging->xc_handle = NULL; /* Tear down domain paging in Xen */ - rc = xc_mem_event_disable(paging->xc_handle, paging->mem_event.domain_id); + rc = xc_mem_event_disable(xch, paging->mem_event.domain_id); if ( rc != 0 ) { ERROR("Error tearing down domain paging in xen"); @@ -277,12 +280,11 @@ int xenpaging_teardown(xc_interface *xch paging->mem_event.xce_handle = -1; /* Close connection to Xen */ - rc = xc_interface_close(paging->xc_handle); + rc = xc_interface_close(xch); if ( rc != 0 ) { ERROR("Error closing connection to xen"); } - paging->xc_handle = NULL; return 0; @@ -336,9 +338,9 @@ static int put_response(mem_event_t *mem return 0; } -int xenpaging_evict_page(xc_interface *xch, xenpaging_t *paging, - xenpaging_victim_t *victim, int fd, int i) +int xenpaging_evict_page(xenpaging_t *paging, xenpaging_victim_t *victim, int fd, int i) { + struct xc_interface *xch = paging->xc_handle; void *page; unsigned long gfn; int ret; @@ -348,7 +350,7 @@ int xenpaging_evict_page(xc_interface *x /* Map page */ gfn = victim->gfn; ret = -EFAULT; - page = xc_map_foreign_pages(paging->xc_handle, victim->domain_id, + page = xc_map_foreign_pages(xch, victim->domain_id, PROT_READ | PROT_WRITE, &gfn, 1); if ( page == NULL ) { @@ -371,7 +373,7 @@ int xenpaging_evict_page(xc_interface *x munmap(page, PAGE_SIZE); /* Tell Xen to evict page */ - ret = xc_mem_paging_evict(paging->xc_handle, paging->mem_event.domain_id, + ret = xc_mem_paging_evict(xch, paging->mem_event.domain_id, victim->gfn); if ( ret != 0 ) { @@ -409,10 +411,9 @@ static int xenpaging_resume_page(xenpagi return ret; } -static int xenpaging_populate_page( - xc_interface *xch, xenpaging_t *paging, - uint64_t *gfn, int fd, int i) +static int xenpaging_populate_page(xenpaging_t *paging, uint64_t *gfn, int fd, int i) { + struct xc_interface *xch = paging->xc_handle; unsigned long _gfn; void *page; int ret; @@ -422,7 +423,7 @@ static int xenpaging_populate_page( do { /* Tell Xen to allocate a page for the domain */ - ret = xc_mem_paging_prep(paging->xc_handle, paging->mem_event.domain_id, + ret = xc_mem_paging_prep(xch, paging->mem_event.domain_id, _gfn); if ( ret != 0 ) { @@ -441,7 +442,7 @@ static int xenpaging_populate_page( /* Map page */ ret = -EFAULT; - page = xc_map_foreign_pages(paging->xc_handle, paging->mem_event.domain_id, + page = xc_map_foreign_pages(xch, paging->mem_event.domain_id, PROT_READ | PROT_WRITE, &_gfn, 1); *gfn = _gfn; if ( page == NULL ) @@ -464,15 +465,16 @@ static int xenpaging_populate_page( return ret; } -static int evict_victim(xc_interface *xch, xenpaging_t *paging, domid_t domain_id, +static int evict_victim(xenpaging_t *paging, domid_t domain_id, xenpaging_victim_t *victim, int fd, int i) { + struct xc_interface *xch = paging->xc_handle; int j = 0; int ret; do { - ret = policy_choose_victim(xch, paging, domain_id, victim); + ret = policy_choose_victim(paging, domain_id, victim); if ( ret != 0 ) { if ( ret != -ENOSPC ) @@ -485,10 +487,10 @@ static int evict_victim(xc_interface *xc ret = -EINTR; goto out; } - ret = xc_mem_paging_nominate(paging->xc_handle, + ret = xc_mem_paging_nominate(xch, paging->mem_event.domain_id, victim->gfn); if ( ret == 0 ) - ret = xenpaging_evict_page(xch, paging, victim, fd, i); + ret = xenpaging_evict_page(paging, victim, fd, i); else { if ( j++ % 1000 == 0 ) @@ -538,13 +540,14 @@ int main(int argc, char *argv[]) srand(time(NULL)); /* Initialise domain paging */ - paging = xenpaging_init(&xch, domain_id); + paging = xenpaging_init(domain_id); if ( paging == NULL ) { - ERROR("Error initialising paging"); + fprintf(stderr, "Error initialising paging"); return 1; } + xch = paging->xc_handle; DPRINTF("starting %s %u %d\n", argv[0], domain_id, num_pages); /* Open file */ @@ -576,7 +579,7 @@ int main(int argc, char *argv[]) memset(victims, 0, sizeof(xenpaging_victim_t) * num_pages); for ( i = 0; i < num_pages; i++ ) { - rc = evict_victim(xch, paging, domain_id, &victims[i], fd, i); + rc = evict_victim(paging, domain_id, &victims[i], fd, i); if ( rc == -ENOSPC ) break; if ( rc == -EINTR ) @@ -629,7 +632,7 @@ int main(int argc, char *argv[]) } /* Populate the page */ - rc = xenpaging_populate_page(xch, paging, &req.gfn, fd, i); + rc = xenpaging_populate_page(paging, &req.gfn, fd, i); if ( rc != 0 ) { ERROR("Error populating page"); @@ -650,7 +653,7 @@ int main(int argc, char *argv[]) } /* Evict a new page to replace the one we just paged in */ - evict_victim(xch, paging, domain_id, &victims[i], fd, i); + evict_victim(paging, domain_id, &victims[i], fd, i); } else { @@ -688,16 +691,14 @@ int main(int argc, char *argv[]) free(victims); /* Tear down domain paging */ - rc1 = xenpaging_teardown(xch, paging); + rc1 = xenpaging_teardown(paging); if ( rc1 != 0 ) - ERROR("Error tearing down paging"); + fprintf(stderr, "Error tearing down paging"); if ( rc == 0 ) rc = rc1; - xc_interface_close(xch); - - DPRINTF("xenpaging exit code %d\n", rc); + printf("xenpaging exit code %d\n", rc); return rc; } _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel