x86/debugger: use copy_to/from_guest() in dbg_rw_guest_mem()

Using gdbsx on Broadwell systems suffers a SMAP violation because
dbg_rw_guest_mem() uses memcpy() with a userspace pointer.

The functions dbg_rw_mem() and dbg_rw_guest_mem() have been updated to pass
'void * __user' pointers which indicates their nature clearly.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff --git a/xen/arch/x86/debug.c b/xen/arch/x86/debug.c
index 435bd40d6c13bf9452d70f142fc733dab60d0c25..801dcf2080fa5f25ff640e8caef4bcfe95c274c7 100644 (file)
--- a/xen/arch/x86/debug.c
+++ b/xen/arch/x86/debug.c
@@ -41,6 +41,9 @@
 #define DBGP2(...) ((void)0)
 #endif
 
+typedef unsigned long dbgva_t;
+typedef unsigned char dbgbyte_t;
+
 /* Returns: mfn for the given (hvm guest) vaddr */
 static unsigned long 
 dbg_hvm_va2mfn(dbgva_t vaddr, struct domain *dp, int toaddr,
@@ -154,13 +157,14 @@ dbg_pv_va2mfn(dbgva_t vaddr, struct domain *dp, uint64_t pgd3val)
 }
 
 /* Returns: number of bytes remaining to be copied */
-static int
-dbg_rw_guest_mem(dbgva_t addr, dbgbyte_t *buf, int len, struct domain *dp, 
-                 int toaddr, uint64_t pgd3)
+unsigned int dbg_rw_guest_mem(struct domain *dp, void * __user gaddr,
+                              void * __user buf, unsigned int len,
+                              bool_t toaddr, uint64_t pgd3)
 {
     while ( len > 0 )
     {
         char *va;
+        unsigned long addr = (unsigned long)gaddr;
         unsigned long mfn, gfn = INVALID_GFN, pagecnt;
 
         pagecnt = min_t(long, PAGE_SIZE - (addr & ~PAGE_MASK), len);
@@ -176,12 +180,12 @@ dbg_rw_guest_mem(dbgva_t addr, dbgbyte_t *buf, int len, struct domain *dp,
 
         if ( toaddr )
         {
-            memcpy(va, buf, pagecnt);    /* va = buf */
+            copy_from_user(va, buf, pagecnt);    /* va = buf */
             paging_mark_dirty(dp, mfn);
         }
         else
         {
-            memcpy(buf, va, pagecnt);    /* buf = va */
+            copy_to_user(buf, va, pagecnt);    /* buf = va */
         }
 
         unmap_domain_page(va);
@@ -203,27 +207,30 @@ dbg_rw_guest_mem(dbgva_t addr, dbgbyte_t *buf, int len, struct domain *dp,
  * pgd3: value of init_mm.pgd[3] in guest. see above.
  * Returns: number of bytes remaining to be copied. 
  */
-int
-dbg_rw_mem(dbgva_t addr, dbgbyte_t *buf, int len, domid_t domid, int toaddr,
-           uint64_t pgd3)
+unsigned int dbg_rw_mem(void * __user addr, void * __user buf,
+                        unsigned int len, domid_t domid, bool_t toaddr,
+                        uint64_t pgd3)
 {
-    struct domain *dp = get_domain_by_id(domid);
-    int hyp = (domid == DOMID_IDLE);
+    DBGP2("gmem:addr:%lx buf:%p len:$%u domid:%d toaddr:%x\n",
+          addr, buf, len, domid, toaddr);
 
-    DBGP2("gmem:addr:%lx buf:%p len:$%d domid:%x toaddr:%x dp:%p\n", 
-          addr, buf, len, domid, toaddr, dp);
-    if ( hyp )
+    if ( domid == DOMID_IDLE )
     {
         if ( toaddr )
-            len = __copy_to_user((void *)addr, buf, len);
+            len = __copy_to_user(addr, buf, len);
         else
-            len = __copy_from_user(buf, (void *)addr, len);
+            len = __copy_from_user(buf, addr, len);
     }
-    else if ( dp )
+    else
     {
-        if ( !dp->is_dying )   /* make sure guest is still there */
-            len= dbg_rw_guest_mem(addr, buf, len, dp, toaddr, pgd3);
-        put_domain(dp);
+        struct domain *d = get_domain_by_id(domid);
+
+        if ( d )
+        {
+            if ( !d->is_dying )
+                len = dbg_rw_guest_mem(d, addr, buf, len, toaddr, pgd3);
+            put_domain(d);
+        }
     }
 
     DBGP2("gmem:exit:len:$%d\n", len);

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index e9f76d02991b7ff9ea1444222babafcae8d3e847..1d3854f51cbc770e3993d480bfa24057804be14c 100644 (file)
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -37,14 +37,14 @@
 #include <asm/debugger.h>
 #include <asm/psr.h>
 
-static int gdbsx_guest_mem_io(
-    domid_t domid, struct xen_domctl_gdbsx_memio *iop)
+static int gdbsx_guest_mem_io(domid_t domid, struct xen_domctl_gdbsx_memio *iop)
 {
-    ulong l_uva = (ulong)iop->uva;
-    iop->remain = dbg_rw_mem(
-        (dbgva_t)iop->gva, (dbgbyte_t *)l_uva, iop->len, domid,
-        iop->gwr, iop->pgd3val);
-    return (iop->remain ? -EFAULT : 0);
+    void * __user gva = (void *)iop->gva, * __user uva = (void *)iop->uva;
+
+    iop->remain = dbg_rw_mem(gva, uva, iop->len, domid,
+                             !!iop->gwr, iop->pgd3val);
+
+    return iop->remain ? -EFAULT : 0;
 }
 
 #define MAX_IOPORTS 0x10000

diff --git a/xen/include/asm-x86/debugger.h b/xen/include/asm-x86/debugger.h
index 0408bec89640ca954b0b2a00a62668c4780e8018..33f47003080755f865edbeb8b7284546e3cae0e1 100644 (file)
--- a/xen/include/asm-x86/debugger.h
+++ b/xen/include/asm-x86/debugger.h
@@ -82,9 +82,8 @@ static inline int debugger_trap_entry(
     return 0;
 }
 
-typedef unsigned long dbgva_t;
-typedef unsigned char dbgbyte_t;
-extern int dbg_rw_mem(dbgva_t addr, dbgbyte_t *buf, int len,
-                      domid_t domid, int toaddr, uint64_t pgd3);
+unsigned int dbg_rw_mem(void * __user addr, void * __user buf,
+                        unsigned int len, domid_t domid, bool_t toaddr,
+                        uint64_t pgd3);
 
 #endif /* __X86_DEBUGGER_H__ */