[Xen-devel] [PATCH v8 20/20] xenctx: Ensure errno is not zero on error in xc_translate_foreign_address()

[Don Slutz <dslutz@xxxxxxxxxxx>]

This is because xc_translate_foreign_address() returns 0 in both the
error case and when 0 is the mfn.  By making sure that errno is set
callers can now determine if it is an error or not.

Fixup map_page() to set errno to zero (0) to improve error handling.

Also call on perror() in map_page().

Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>
---
 tools/libxc/xc_pagetab.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
 tools/xentrace/xenctx.c  | 22 +++++++++++++++++++---
 2 files changed, 60 insertions(+), 10 deletions(-)

diff --git a/tools/libxc/xc_pagetab.c b/tools/libxc/xc_pagetab.c
index 9d3349f..5c8049b 100644
--- a/tools/libxc/xc_pagetab.c
+++ b/tools/libxc/xc_pagetab.c
@@ -35,12 +35,17 @@ unsigned long xc_translate_foreign_address(xc_interface 
*xch, uint32_t dom,
     int size, level, pt_levels = 2;
     void *map;
 
-    if (xc_domain_getinfo(xch, dom, 1, &dominfo) != 1 
-        || dominfo.domid != dom)
+    if ( xc_domain_getinfo(xch, dom, 1, &dominfo) != 1
+         || dominfo.domid != dom )
+    {
+        if ( !errno )
+            errno = EINVAL;
         return 0;
+    }
 
     /* What kind of paging are we dealing with? */
-    if (dominfo.hvm) {
+    if ( dominfo.hvm )
+    {
         struct hvm_hw_cpu hvm_ctx;
         if (xc_domain_hvm_getcontext_partial(xch, dom,
                                              HVM_SAVE_CODE(CPU), vcpu,
@@ -50,14 +55,26 @@ unsigned long xc_translate_foreign_address(xc_interface 
*xch, uint32_t dom,
             vcpu_guest_context_any_t ctx;
 
             if ( errno != EBADSLT )
+            {
+                if ( !errno )
+                    errno = EINVAL;
                 return 0;
+            }
             /*
              * Offline CPU, use xc_vcpu_getcontext() if possible
              */
             if ( xc_vcpu_getcontext(xch, dom, vcpu, &ctx) != 0 )
+            {
+                if ( !errno )
+                    errno = EINVAL;
                 return 0;
+            }
             if ( xc_domain_get_guest_width(xch, dom, &gwidth) != 0 )
+            {
+                if ( !errno )
+                    errno = EINVAL;
                 return 0;
+            }
             if ( gwidth == 8 )
             {
                 if ( !(ctx.x64.ctrlreg[0] & CR0_PG) )
@@ -85,10 +102,18 @@ unsigned long xc_translate_foreign_address(xc_interface 
*xch, uint32_t dom,
     } else {
         unsigned int gwidth;
         vcpu_guest_context_any_t ctx;
-        if (xc_vcpu_getcontext(xch, dom, vcpu, &ctx) != 0)
+        if ( xc_vcpu_getcontext(xch, dom, vcpu, &ctx) != 0 )
+        {
+            if ( !errno )
+                errno = EINVAL;
             return 0;
+        }
         if (xc_domain_get_guest_width(xch, dom, &gwidth) != 0)
+        {
+            if ( !errno )
+                errno = EINVAL;
             return 0;
+        }
         if (gwidth == 8) {
             pt_levels = 4;
             paddr = (uint64_t)xen_cr3_to_pfn_x86_64(ctx.x64.ctrlreg[3])
@@ -117,14 +142,23 @@ unsigned long xc_translate_foreign_address(xc_interface 
*xch, uint32_t dom,
         paddr += ((virt & mask) >> (xc_ffs64(mask) - 1)) * size;
         map = xc_map_foreign_range(xch, dom, PAGE_SIZE, PROT_READ, 
                                    paddr >>PAGE_SHIFT);
-        if (!map) 
+        if ( !map )
+        {
+            if ( !errno )
+                errno = EINVAL;
             return 0;
+        }
         memcpy(&pte, map + (paddr & (PAGE_SIZE - 1)), size);
         munmap(map, PAGE_SIZE);
-        if (!(pte & 1)) 
+        if ( !(pte & 1) )
+        {
+            if ( !errno )
+                errno = EADDRNOTAVAIL;
             return 0;
+        }
         paddr = pte & 0x000ffffffffff000ull;
-        if (level == 2 && (pte & PTE_PSE)) {
+        if ( level == 2 && (pte & PTE_PSE) )
+        {
             mask = ((mask ^ ~-mask) >> 1); /* All bits below first set bit */
             return ((paddr & ~mask) | (virt & mask)) >> PAGE_SHIFT;
         }
diff --git a/tools/xentrace/xenctx.c b/tools/xentrace/xenctx.c
index 6105ffb..f485ade 100644
--- a/tools/xentrace/xenctx.c
+++ b/tools/xentrace/xenctx.c
@@ -713,21 +713,37 @@ static void *map_page(vcpu_guest_context_any_t *ctx, int 
vcpu, guest_word_t virt
     static unsigned long previous_mfn = 0;
     static void *mapped = NULL;
 
-    unsigned long mfn = xc_translate_foreign_address(xenctx.xc_handle, 
xenctx.domid, vcpu, virt);
+    unsigned long mfn;
     unsigned long offset = virt & ~XC_PAGE_MASK;
+    int saved_errno;
 
-    if (mapped && mfn == previous_mfn)
+    errno = 0;
+    mfn = xc_translate_foreign_address(xenctx.xc_handle, xenctx.domid,
+                                       vcpu, virt);
+    saved_errno = errno;
+    if ( mapped && mfn == previous_mfn && !saved_errno )
         goto out;
 
     if (mapped)
         munmap(mapped, XC_PAGE_SIZE);
 
+    if ( saved_errno )
+    {
+        fprintf(stderr, "\nfailed to map page for "FMT_32B_WORD".\n", virt);
+        errno = saved_errno;
+        perror("xenctx");
+        return NULL;
+    }
     previous_mfn = mfn;
 
     mapped = xc_map_foreign_range(xenctx.xc_handle, xenctx.domid, 
XC_PAGE_SIZE, PROT_READ, mfn);
 
-    if (mapped == NULL) {
+    if ( mapped == NULL )
+    {
+        saved_errno = errno;
         fprintf(stderr, "\nfailed to map page for "FMT_32B_WORD".\n", virt);
+        errno = saved_errno;
+        perror("xenctx");
         return NULL;
     }
 
-- 
1.8.4


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