From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: [PATCH v2] tools/libxc: Prevent erroneous success from xc_domain_restore Date: Tue, 4 Feb 2014 18:01:10 +0000 Message-ID: <1391536870-22809-1-git-send-email-andrew.cooper3@citrix.com> References: <1391535813.6497.61.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1391535813.6497.61.camel@kazak.uk.xensource.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 Cc: George Dunlap , Andrew Cooper , Ian Jackson , Ian Campbell List-Id: xen-devel@lists.xenproject.org The variable 'rc' is set to 1 at the top of xc_domain_restore, and for the most part is left alone until success, at which point it is set to 0. There is a separate 'frc' which for the most part is used to check function calls, keeping errors separate from 'rc'. For a toolstack which sets callbacks->toolstack_restore(), and the function returns 0, any subsequent error will end up with code flow going to "out;", resulting in the migration being declared a success. For consistency, update the callsites of xc_dom_gnttab{,_hvm}_seed() to use 'frc', even though their use of 'rc' is currently safe. Signed-off-by: Andrew Cooper CC: Ian Campbell CC: Ian Jackson CC: George Dunlap --- Changes in v2: * Dont drop rc = -1 from toolstack_restore(). Regarding 4.4: If the two "for consistency" changes to xc_dom_gnttab{,_hvm}_seed() are considered too risky, they can be dropped without affecting the bugfix nature of the patch, but I would argue that leaving some examples of "rc = function_call()" leaves a bad precident which is likely to lead to similar bugs in the future. --- tools/libxc/xc_domain_restore.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c index 5ba47d7..1f6ce50 100644 --- a/tools/libxc/xc_domain_restore.c +++ b/tools/libxc/xc_domain_restore.c @@ -2240,9 +2240,9 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, memcpy(ctx->live_p2m, ctx->p2m, dinfo->p2m_size * sizeof(xen_pfn_t)); munmap(ctx->live_p2m, P2M_FL_ENTRIES * PAGE_SIZE); - rc = xc_dom_gnttab_seed(xch, dom, *console_mfn, *store_mfn, - console_domid, store_domid); - if (rc != 0) + frc = xc_dom_gnttab_seed(xch, dom, *console_mfn, *store_mfn, + console_domid, store_domid); + if (frc != 0) { ERROR("error seeding grant table"); goto out; @@ -2257,10 +2257,10 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, { if ( callbacks != NULL && callbacks->toolstack_restore != NULL ) { - rc = callbacks->toolstack_restore(dom, tdata.data, tdata.len, - callbacks->data); + frc = callbacks->toolstack_restore(dom, tdata.data, tdata.len, + callbacks->data); free(tdata.data); - if ( rc < 0 ) + if ( frc < 0 ) { PERROR("error calling toolstack_restore"); goto out; @@ -2326,9 +2326,9 @@ int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom, goto out; } - rc = xc_dom_gnttab_hvm_seed(xch, dom, *console_mfn, *store_mfn, - console_domid, store_domid); - if (rc != 0) + frc = xc_dom_gnttab_hvm_seed(xch, dom, *console_mfn, *store_mfn, + console_domid, store_domid); + if (frc != 0) { ERROR("error seeding grant table"); goto out; -- 1.7.10.4