Re: [Xen-devel] [PATCH 19/22] libxc: check failure of xc_dom_*_to_ptr, xc_map_foreign_range

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Jackson writes ("[PATCH 19/22] libxc: check failure of xc_dom_*_to_ptr, 
xc_map_foreign_range"):
> The return values from xc_dom_*_to_ptr and xc_map_foreign_range are
> sometimes dereferenced, or subjected to pointer arithmetic, without
> checking whether the relevant function failed and returned NULL.
> 
> Add an appropriate error check at every call site.

I seem to have missed one.  I intend to add this to v8 of the series.

commit 029d07d15fb27ab4b107c961b7ad74701ea623a8
Author: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Date:   Thu Jun 13 16:19:58 2013 +0100

    xc_exchange_page

diff --git a/.topmsg b/.topmsg
index f211f1f..4ca5d4f 100644
--- a/.topmsg
+++ b/.topmsg
@@ -12,6 +12,12 @@ This is part of the fix to a security issue, XSA-55.
 Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
 Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
 
+v8: Add a missing check in xc_offline_page.c:xc_exchange_page.
+     I think in this particular error case it may be that the results
+     are a broken guest, but turning this from a possible host tools
+     crash into a guest problem seems to solve the potential security
+     problem.
+
 v7: Simplify an error DOMPRINTF to not use "load ? : ".
     Make DOMPRINTF allocation error messages consistent.
     Do not set elf->dest_pages in xc_dom_load_elf_kernel
diff --git a/tools/libxc/xc_offline_page.c b/tools/libxc/xc_offline_page.c
index 089a361..36b9812 100644
--- a/tools/libxc/xc_offline_page.c
+++ b/tools/libxc/xc_offline_page.c
@@ -714,6 +714,11 @@ int xc_exchange_page(xc_interface *xch, int domid, 
xen_pfn_t mfn)
 
         new_p = xc_map_foreign_range(xch, domid, PAGE_SIZE,
                                      PROT_READ|PROT_WRITE, new_mfn);
+        if ( new_p == NULL )
+        {
+            ERROR("failed to map new_p for copy, guest may be broken?");
+            goto failed;
+        }
         memcpy(new_p, backup, PAGE_SIZE);
         munmap(new_p, PAGE_SIZE);
         mops.arg1.mfn = new_mfn;

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