[Xen-devel] [PATCH] x86/dmop: Fix compat_dm_op() ABI

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

The parameter to compat_dm_op() is a pointer to an array of
compat_dm_op_buf_t's in guest RAM.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Paul Durrant <paul.durrant@xxxxxxxxxx>
CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>

What is the pupose of COMPAT_HANDLE_PARAM()? It is a packed structure of one
and a half pointers, so isn't safe at all for use in the hypercall function
APIs (depsite its naming making it look deceptively like it is the correct
thing to use).

On a more serious note, why do we have all this macro infrastrucutre in the
first place?  Having spent rather longer debugging this than I to admit
(almost mainly from the userspace side) I have concluded that it is actively
dangerous to use; all it does is hide what is going on.

What does it actually give us that the Linux route of a real C pointers and a
__user attribute doesn't, other than obfuscating the code on the hypercall
boundary?
---
 xen/arch/x86/hvm/dm.c       | 4 ++--
 xen/include/xen/hypercall.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
index 6a722a5..2122c45 100644
--- a/xen/arch/x86/hvm/dm.c
+++ b/xen/arch/x86/hvm/dm.c
@@ -525,7 +525,7 @@ CHECK_dm_op_inject_msi;
 
 int compat_dm_op(domid_t domid,
                  unsigned int nr_bufs,
-                 COMPAT_HANDLE_PARAM(compat_dm_op_buf_t) bufs)
+                 XEN_GUEST_HANDLE_PARAM(void) bufs)
 {
     struct xen_dm_op_buf nat[MAX_NR_BUFS];
     unsigned int i;
@@ -538,7 +538,7 @@ int compat_dm_op(domid_t domid,
     {
         struct compat_dm_op_buf cmp;
 
-        if ( copy_from_compat_offset(&cmp, bufs, i, 1) )
+        if ( copy_from_guest_offset(&cmp, bufs, i, 1) )
             return -EFAULT;
 
 #define XLAT_dm_op_buf_HNDL_h(_d_, _s_) \
diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
index 8d4824f..cc99aea 100644
--- a/xen/include/xen/hypercall.h
+++ b/xen/include/xen/hypercall.h
@@ -203,7 +203,7 @@ extern int
 compat_dm_op(
     domid_t domid,
     unsigned int nr_bufs,
-    COMPAT_HANDLE_PARAM(compat_dm_op_buf_t) bufs);
+    XEN_GUEST_HANDLE_PARAM(void) bufs);
 
 #endif
 
-- 
2.1.4


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