[PATCH v5] x86/altp2m: Added xc_altp2m_set_mem_access_multi()

[PATCH v5] x86/altp2m: Added xc_altp2m_set_mem_access_multi()

[Petre Pircalabu]

From: Razvan Cojocaru <rcojocaru@bitdefender.com>

For the default EPT view we have xc_set_mem_access_multi(), which
is able to set an array of pages to an array of access rights with
a single hypercall. However, this functionality was lacking for the
altp2m subsystem, which could only set page restrictions for one
page at a time. This patch addresses the gap.

HVMOP_altp2m_set_mem_access_multi has been added as a HVMOP (as opposed to a
DOMCTL) for consistency with its HVMOP_altp2m_set_mem_access counterpart (and
hence with the original altp2m design, where domains are allowed - with the
proper altp2m access rights - to alter these settings), in the absence of an
official position on the issue from the original altp2m designers.

Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>

---

Changed since v2:
    * Added support for compat arguments translation

Changed since v3:
    * Replaced  __copy_to_guest with __copy_field_to_guest
    * Removed the un-needed parentheses.
    * Fixed xlat.lst ordering
    * Added comment to patch description explaining why the
    functionality was added as an HVMOP.
    * Guard using XEN_GENERATING_COMPAT_HEADERS the hvmmem_type_t definition.
    This will prevent suplicate definitions to be generated for the
    compat equivalent.
    * Added comment describing the manual translation of
    xen_hvm_altp2m_op_t generic fields from compat_hvm_altp2m_op_t.

Changed since v4:
    * Changed the mask parameter to 0x3Fa.
    * Split long lines.
    * Added "improperly named HVMMEM_(*)" to the comment explaining the
    XEN_GENERATING_COMPAT_HEADERS guard.
    * Removed typedef and XEN_GUEST_HANDLE for xen_hvm_altp2m_set_mem_access_multi.
    * Copied the "opaque" field to guest in compat_altp2m_op.
    * Added build time test to check if the size of
    xen_hvm_altp2m_set_mem_access_multi at least equal to the size of
    compat_hvm_altp2m_set_mem_access_multi.
---
 tools/libxc/include/xenctrl.h   |   3 ++
 tools/libxc/xc_altp2m.c         |  41 ++++++++++++++++
 xen/arch/x86/hvm/hvm.c          | 105 +++++++++++++++++++++++++++++++++++++++-
 xen/include/Makefile            |   1 +
 xen/include/public/hvm/hvm_op.h |  36 ++++++++++++--
 xen/include/xlat.lst            |   1 +
 6 files changed, 181 insertions(+), 6 deletions(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 3bcab3c..4e2ce64 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1971,6 +1971,9 @@ int xc_altp2m_switch_to_view(xc_interface *handle, domid_t domid,
 int xc_altp2m_set_mem_access(xc_interface *handle, domid_t domid,
                              uint16_t view_id, xen_pfn_t gfn,
                              xenmem_access_t access);
+int xc_altp2m_set_mem_access_multi(xc_interface *handle, domid_t domid,
+                                   uint16_t view_id, uint8_t *access,
+                                   uint64_t *pages, uint32_t nr);
 int xc_altp2m_change_gfn(xc_interface *handle, domid_t domid,
                          uint16_t view_id, xen_pfn_t old_gfn,
                          xen_pfn_t new_gfn);
diff --git a/tools/libxc/xc_altp2m.c b/tools/libxc/xc_altp2m.c
index 0639632..f202ca1 100644
--- a/tools/libxc/xc_altp2m.c
+++ b/tools/libxc/xc_altp2m.c
@@ -188,6 +188,47 @@ int xc_altp2m_set_mem_access(xc_interface *handle, domid_t domid,
     return rc;
 }
 
+int xc_altp2m_set_mem_access_multi(xc_interface *xch, domid_t domid,
+                                   uint16_t view_id, uint8_t *access,
+                                   uint64_t *pages, uint32_t nr)
+{
+    int rc;
+
+    DECLARE_HYPERCALL_BUFFER(xen_hvm_altp2m_op_t, arg);
+    DECLARE_HYPERCALL_BOUNCE(access, nr, XC_HYPERCALL_BUFFER_BOUNCE_IN);
+    DECLARE_HYPERCALL_BOUNCE(pages, nr * sizeof(uint64_t),
+                             XC_HYPERCALL_BUFFER_BOUNCE_IN);
+
+    arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
+    if ( arg == NULL )
+        return -1;
+
+    arg->version = HVMOP_ALTP2M_INTERFACE_VERSION;
+    arg->cmd = HVMOP_altp2m_set_mem_access_multi;
+    arg->domain = domid;
+    arg->u.set_mem_access_multi.view = view_id;
+    arg->u.set_mem_access_multi.nr = nr;
+
+    if ( xc_hypercall_bounce_pre(xch, pages) ||
+         xc_hypercall_bounce_pre(xch, access) )
+    {
+        PERROR("Could not bounce memory for HVMOP_altp2m_set_mem_access_multi");
+        return -1;
+    }
+
+    set_xen_guest_handle(arg->u.set_mem_access_multi.pfn_list, pages);
+    set_xen_guest_handle(arg->u.set_mem_access_multi.access_list, access);
+
+    rc = xencall2(xch->xcall, __HYPERVISOR_hvm_op, HVMOP_altp2m,
+		  HYPERCALL_BUFFER_AS_ARG(arg));
+
+    xc_hypercall_buffer_free(xch, arg);
+    xc_hypercall_bounce_post(xch, access);
+    xc_hypercall_bounce_post(xch, pages);
+
+    return rc;
+}
+
 int xc_altp2m_change_gfn(xc_interface *handle, domid_t domid,
                          uint16_t view_id, xen_pfn_t old_gfn,
                          xen_pfn_t new_gfn)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 205b4cb..4bf8b32 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -73,6 +73,8 @@
 #include <public/arch-x86/cpuid.h>
 #include <asm/cpuid.h>
 
+#include <compat/hvm/hvm_op.h>
+
 bool_t __read_mostly hvm_enabled;
 
 #ifdef DBG_LEVEL_0
@@ -4451,6 +4453,7 @@ static int do_altp2m_op(
     case HVMOP_altp2m_destroy_p2m:
     case HVMOP_altp2m_switch_p2m:
     case HVMOP_altp2m_set_mem_access:
+    case HVMOP_altp2m_set_mem_access_multi:
     case HVMOP_altp2m_change_gfn:
         break;
     default:
@@ -4568,6 +4571,32 @@ static int do_altp2m_op(
                                     a.u.set_mem_access.view);
         break;
 
+    case HVMOP_altp2m_set_mem_access_multi:
+        if ( a.u.set_mem_access_multi.pad ||
+             a.u.set_mem_access_multi.opaque >= a.u.set_mem_access_multi.nr )
+        {
+            rc = -EINVAL;
+            break;
+        }
+
+        rc = p2m_set_mem_access_multi(d, a.u.set_mem_access_multi.pfn_list,
+                                      a.u.set_mem_access_multi.access_list,
+                                      a.u.set_mem_access_multi.nr,
+                                      a.u.set_mem_access_multi.opaque,
+                                      0x3F,
+                                      a.u.set_mem_access_multi.view);
+        if ( rc > 0 )
+        {
+            a.u.set_mem_access_multi.opaque = rc;
+            if ( __copy_field_to_guest(guest_handle_cast(arg, xen_hvm_altp2m_op_t),
+                                       &a, u.set_mem_access_multi.opaque) )
+                rc = -EFAULT;
+            else
+                rc = hypercall_create_continuation(__HYPERVISOR_hvm_op, "lh",
+                                                   HVMOP_altp2m, arg);
+        }
+        break;
+
     case HVMOP_altp2m_change_gfn:
         if ( a.u.change_gfn.pad1 || a.u.change_gfn.pad2 )
             rc = -EINVAL;
@@ -4586,6 +4615,80 @@ static int do_altp2m_op(
     return rc;
 }
 
+DEFINE_XEN_GUEST_HANDLE(compat_hvm_altp2m_op_t);
+
+static int compat_altp2m_op(
+    XEN_GUEST_HANDLE_PARAM(void) arg)
+{
+    int rc = 0;
+    struct compat_hvm_altp2m_op a;
+    union
+    {
+        XEN_GUEST_HANDLE_PARAM(void) hnd;
+        struct xen_hvm_altp2m_op *altp2m_op;
+    } nat;
+
+    if ( !hvm_altp2m_supported() )
+        return -EOPNOTSUPP;
+
+    if ( copy_from_guest(&a, arg, 1) )
+        return -EFAULT;
+
+    if ( a.pad1 || a.pad2 ||
+         (a.version != HVMOP_ALTP2M_INTERFACE_VERSION) )
+        return -EINVAL;
+
+    set_xen_guest_handle(nat.hnd, COMPAT_ARG_XLAT_VIRT_BASE);
+
+    switch ( a.cmd )
+    {
+        case HVMOP_altp2m_set_mem_access_multi:
+            BUILD_BUG_ON(sizeof(struct xen_hvm_altp2m_set_mem_access_multi) <
+                         sizeof(struct compat_hvm_altp2m_set_mem_access_multi));
+#define XLAT_hvm_altp2m_set_mem_access_multi_HNDL_pfn_list(_d_, _s_); \
+            guest_from_compat_handle((_d_)->pfn_list, (_s_)->pfn_list)
+#define XLAT_hvm_altp2m_set_mem_access_multi_HNDL_access_list(_d_, _s_); \
+            guest_from_compat_handle((_d_)->access_list, (_s_)->access_list)
+            XLAT_hvm_altp2m_set_mem_access_multi(&nat.altp2m_op->u.set_mem_access_multi,
+                    &a.u.set_mem_access_multi);
+#undef XLAT_hvm_altp2m_set_mem_access_multi_HNDL_pfn_list
+#undef XLAT_hvm_altp2m_set_mem_access_multi_HNDL_access_list
+            break;
+        default:
+            return do_altp2m_op(arg);
+    }
+
+    /*
+     * Manually fill the common part of the xen_hvm_altp2m_op structure because
+     * the generated XLAT_hvm_altp2m_op macro doesn't correctly handle the
+     * translation of all fields from compat_hvm_altp2m_op to xen_hvm_altp2m_op.
+     */
+    nat.altp2m_op->version  = a.version;
+    nat.altp2m_op->cmd      = a.cmd;
+    nat.altp2m_op->domain   = a.domain;
+    nat.altp2m_op->pad1     = a.pad1;
+    nat.altp2m_op->pad2     = a.pad2;
+
+    rc = do_altp2m_op(nat.hnd);
+
+    switch ( a.cmd )
+    {
+        case HVMOP_altp2m_set_mem_access_multi:
+            if ( nat.altp2m_op->u.set_mem_access_multi.opaque > 0 )
+            {
+                a.u.set_mem_access_multi.opaque =
+                    nat.altp2m_op->u.set_mem_access_multi.opaque;
+                if ( __copy_field_to_guest(guest_handle_cast(arg,
+                                                             compat_hvm_altp2m_op_t),
+                                           &a, u.set_mem_access_multi.opaque) )
+                    rc = -EFAULT;
+            }
+            break;
+    }
+
+    return rc;
+}
+
 static int hvmop_get_mem_type(
     XEN_GUEST_HANDLE_PARAM(xen_hvm_get_mem_type_t) arg)
 {
@@ -4733,7 +4836,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
         break;
 
     case HVMOP_altp2m:
-        rc = do_altp2m_op(arg);
+        rc = current->hcall_compat ? compat_altp2m_op(arg) : do_altp2m_op(arg);
         break;
 
     default:
diff --git a/xen/include/Makefile b/xen/include/Makefile
index 1299b19..8fc6e2b 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -28,6 +28,7 @@ headers-$(CONFIG_X86)     += compat/arch-x86/xen.h
 headers-$(CONFIG_X86)     += compat/arch-x86/xen-$(compat-arch-y).h
 headers-$(CONFIG_X86)     += compat/hvm/hvm_vcpu.h
 headers-$(CONFIG_X86)     += compat/hvm/dm_op.h
+headers-$(CONFIG_X86)     += compat/hvm/hvm_op.h
 headers-y                 += compat/arch-$(compat-arch-y).h compat/pmu.h compat/xlat.h
 headers-$(CONFIG_FLASK)   += compat/xsm/flask_op.h
 
diff --git a/xen/include/public/hvm/hvm_op.h b/xen/include/public/hvm/hvm_op.h
index 0bdafdf..12de88ac 100644
--- a/xen/include/public/hvm/hvm_op.h
+++ b/xen/include/public/hvm/hvm_op.h
@@ -83,6 +83,13 @@ DEFINE_XEN_GUEST_HANDLE(xen_hvm_set_pci_link_route_t);
 /* Flushes all VCPU TLBs: @arg must be NULL. */
 #define HVMOP_flush_tlbs          5
 
+/*
+ * hvmmem_type_t should not be defined when generating the corresponding
+ * compat header. This will ensure that the improperly named HVMMEM_(*)
+ * values are defined only once.
+ */
+#ifndef XEN_GENERATING_COMPAT_HEADERS
+
 typedef enum {
     HVMMEM_ram_rw,             /* Normal read/write guest RAM */
     HVMMEM_ram_ro,             /* Read-only; writes are discarded */
@@ -102,6 +109,8 @@ typedef enum {
                                   to HVMMEM_ram_rw. */
 } hvmmem_type_t;
 
+#endif /* XEN_GENERATING_COMPAT_HEADERS */
+
 /* Hint from PV drivers for pagetable destruction. */
 #define HVMOP_pagetable_dying        9
 struct xen_hvm_pagetable_dying {
@@ -237,6 +246,20 @@ struct xen_hvm_altp2m_set_mem_access {
 typedef struct xen_hvm_altp2m_set_mem_access xen_hvm_altp2m_set_mem_access_t;
 DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_set_mem_access_t);
 
+struct xen_hvm_altp2m_set_mem_access_multi {
+    /* view */
+    uint16_t view;
+    uint16_t pad;
+    /* Number of pages */
+    uint32_t nr;
+    /* Used for continuation purposes */
+    uint64_t opaque;
+    /* List of pfns to set access for */
+    XEN_GUEST_HANDLE(const_uint64) pfn_list;
+    /* Corresponding list of access settings for pfn_list */
+    XEN_GUEST_HANDLE(const_uint8) access_list;
+};
+
 struct xen_hvm_altp2m_change_gfn {
     /* view */
     uint16_t view;
@@ -268,15 +291,18 @@ struct xen_hvm_altp2m_op {
 #define HVMOP_altp2m_set_mem_access       7
 /* Change a p2m entry to have a different gfn->mfn mapping */
 #define HVMOP_altp2m_change_gfn           8
+/* Set access for an array of pages */
+#define HVMOP_altp2m_set_mem_access_multi 9
     domid_t domain;
     uint16_t pad1;
     uint32_t pad2;
     union {
-        struct xen_hvm_altp2m_domain_state       domain_state;
-        struct xen_hvm_altp2m_vcpu_enable_notify enable_notify;
-        struct xen_hvm_altp2m_view               view;
-        struct xen_hvm_altp2m_set_mem_access     set_mem_access;
-        struct xen_hvm_altp2m_change_gfn         change_gfn;
+        struct xen_hvm_altp2m_domain_state         domain_state;
+        struct xen_hvm_altp2m_vcpu_enable_notify   enable_notify;
+        struct xen_hvm_altp2m_view                 view;
+        struct xen_hvm_altp2m_set_mem_access       set_mem_access;
+        struct xen_hvm_altp2m_change_gfn           change_gfn;
+        struct xen_hvm_altp2m_set_mem_access_multi set_mem_access_multi;
         uint8_t pad[64];
     } u;
 };
diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
index 0f17000..5010fcc 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -70,6 +70,7 @@
 ?	dm_op_set_pci_intx_level	hvm/dm_op.h
 ?	dm_op_set_pci_link_route	hvm/dm_op.h
 ?	dm_op_track_dirty_vram		hvm/dm_op.h
+!	hvm_altp2m_set_mem_access_multi	hvm/hvm_op.h
 ?	vcpu_hvm_context		hvm/hvm_vcpu.h
 ?	vcpu_hvm_x86_32			hvm/hvm_vcpu.h
 ?	vcpu_hvm_x86_64			hvm/hvm_vcpu.h
-- 
2.7.4


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

Re: [PATCH v5] x86/altp2m: Added xc_altp2m_set_mem_access_multi()

[Jan Beulich]

>>> On 11.10.17 at 18:26, <ppircalabu@bitdefender.com> wrote:
> @@ -4568,6 +4571,32 @@ static int do_altp2m_op(
>                                      a.u.set_mem_access.view);
>          break;
>  
> +    case HVMOP_altp2m_set_mem_access_multi:
> +        if ( a.u.set_mem_access_multi.pad ||
> +             a.u.set_mem_access_multi.opaque >= a.u.set_mem_access_multi.nr )
> +        {
> +            rc = -EINVAL;
> +            break;
> +        }
> +
> +        rc = p2m_set_mem_access_multi(d, a.u.set_mem_access_multi.pfn_list,
> +                                      a.u.set_mem_access_multi.access_list,
> +                                      a.u.set_mem_access_multi.nr,
> +                                      a.u.set_mem_access_multi.opaque,
> +                                      0x3F,

Please add /* pretty arbitrary */ or something similar here.

> @@ -4586,6 +4615,80 @@ static int do_altp2m_op(
>      return rc;
>  }
>  
> +DEFINE_XEN_GUEST_HANDLE(compat_hvm_altp2m_op_t);
> +
> +static int compat_altp2m_op(
> +    XEN_GUEST_HANDLE_PARAM(void) arg)
> +{
> +    int rc = 0;
> +    struct compat_hvm_altp2m_op a;
> +    union
> +    {
> +        XEN_GUEST_HANDLE_PARAM(void) hnd;
> +        struct xen_hvm_altp2m_op *altp2m_op;
> +    } nat;
> +
> +    if ( !hvm_altp2m_supported() )
> +        return -EOPNOTSUPP;
> +
> +    if ( copy_from_guest(&a, arg, 1) )
> +        return -EFAULT;
> +
> +    if ( a.pad1 || a.pad2 ||
> +         (a.version != HVMOP_ALTP2M_INTERFACE_VERSION) )
> +        return -EINVAL;
> +
> +    set_xen_guest_handle(nat.hnd, COMPAT_ARG_XLAT_VIRT_BASE);
> +
> +    switch ( a.cmd )
> +    {
> +        case HVMOP_altp2m_set_mem_access_multi:

Indentation.

> +            BUILD_BUG_ON(sizeof(struct xen_hvm_altp2m_set_mem_access_multi) <
> +                         sizeof(struct compat_hvm_altp2m_set_mem_access_multi));

What good does this do?

> +#define XLAT_hvm_altp2m_set_mem_access_multi_HNDL_pfn_list(_d_, _s_); \
> +            guest_from_compat_handle((_d_)->pfn_list, (_s_)->pfn_list)
> +#define XLAT_hvm_altp2m_set_mem_access_multi_HNDL_access_list(_d_, _s_); \
> +            guest_from_compat_handle((_d_)->access_list, (_s_)->access_list)
> +            XLAT_hvm_altp2m_set_mem_access_multi(&nat.altp2m_op->u.set_mem_access_multi,
> +                    &a.u.set_mem_access_multi);
> +#undef XLAT_hvm_altp2m_set_mem_access_multi_HNDL_pfn_list
> +#undef XLAT_hvm_altp2m_set_mem_access_multi_HNDL_access_list
> +            break;
> +        default:
> +            return do_altp2m_op(arg);
> +    }
> +
> +    /*
> +     * Manually fill the common part of the xen_hvm_altp2m_op structure because
> +     * the generated XLAT_hvm_altp2m_op macro doesn't correctly handle the
> +     * translation of all fields from compat_hvm_altp2m_op to xen_hvm_altp2m_op.
> +     */
> +    nat.altp2m_op->version  = a.version;
> +    nat.altp2m_op->cmd      = a.cmd;
> +    nat.altp2m_op->domain   = a.domain;
> +    nat.altp2m_op->pad1     = a.pad1;
> +    nat.altp2m_op->pad2     = a.pad2;

There are _still_ no size checks here.

> +    rc = do_altp2m_op(nat.hnd);
> +
> +    switch ( a.cmd )
> +    {
> +        case HVMOP_altp2m_set_mem_access_multi:

Indentation.

> +            if ( nat.altp2m_op->u.set_mem_access_multi.opaque > 0 )

Please also check rc here to avoid needlessly copying back. In
fact _only_ checking rc ought to be fine.

> +            {
> +                a.u.set_mem_access_multi.opaque =
> +                    nat.altp2m_op->u.set_mem_access_multi.opaque;

You also need a size check here.

Jan


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

Re: [PATCH v5] x86/altp2m: Added xc_altp2m_set_mem_access_multi()

[Petre Ovidiu PIRCALABU]

On Vi, 2017-10-13 at 03:51 -0600, Jan Beulich wrote:
> > 
> > 
> > +            BUILD_BUG_ON(sizeof(struct
> > xen_hvm_altp2m_set_mem_access_multi) <
> > +                         sizeof(struct
> > compat_hvm_altp2m_set_mem_access_multi));
> What good does this do?
> Sorry, I don't understand how these size checks should look (are we
> testing that the left hand side is at least as wide as the right hand
> side?). Could you please give an example? Thanks!

> > +     */
> > +    nat.altp2m_op->version  = a.version;
> > +    nat.altp2m_op->cmd      = a.cmd;
> > +    nat.altp2m_op->domain   = a.domain;
> > +    nat.altp2m_op->pad1     = a.pad1;
> > +    nat.altp2m_op->pad2     = a.pad2;
> There are _still_ no size checks here.
I'm not sure I understand what kind of size checks should be used here.
Are you expecting something similar with the CHECK_FIELD_ macro?
> > 
> > +            if ( nat.altp2m_op->u.set_mem_access_multi.opaque > 0
> > )
> Please also check rc here to avoid needlessly copying back. In
> fact _only_ checking rc ought to be fine.
Actually, in this case rc is overwritten in do_altp2m_op (-EFAULT if
the copy didn't work or the result of hypercall_create_continuation
otherwise).
I have used the check for nat.altp2m_op->u.set_mem_access_multi.opaque
as it usually gets set to a positive value only if the hypercall gets
preempted,

Many thanks for your support,
Petre
> 
> > 
> 
> Jan
> 
> 
> ________________________
> This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v5] x86/altp2m: Added xc_altp2m_set_mem_access_multi()

[Jan Beulich]

>>> On 13.10.17 at 15:00, <ppircalabu@bitdefender.com> wrote:
> On Vi, 2017-10-13 at 03:51 -0600, Jan Beulich wrote:
>> > 
>> > 
>> > +            BUILD_BUG_ON(sizeof(struct
>> > xen_hvm_altp2m_set_mem_access_multi) <
>> > +                         sizeof(struct
>> > compat_hvm_altp2m_set_mem_access_multi));
>> What good does this do?
>> Sorry, I don't understand how these size checks should look (are we
>> testing that the left hand side is at least as wide as the right hand
>> side?). Could you please give an example? Thanks!
> 
>> > +     */
>> > +    nat.altp2m_op->version  = a.version;
>> > +    nat.altp2m_op->cmd      = a.cmd;
>> > +    nat.altp2m_op->domain   = a.domain;
>> > +    nat.altp2m_op->pad1     = a.pad1;
>> > +    nat.altp2m_op->pad2     = a.pad2;
>> There are _still_ no size checks here.
> I'm not sure I understand what kind of size checks should be used here.
> Are you expecting something similar with the CHECK_FIELD_ macro?

Yes, as I've been stating repeatedly. You want to demonstrate
that field sizes match, or that at least no truncation can occur.

>> > 
>> > +            if ( nat.altp2m_op->u.set_mem_access_multi.opaque > 0
>> > )
>> Please also check rc here to avoid needlessly copying back. In
>> fact _only_ checking rc ought to be fine.
> Actually, in this case rc is overwritten in do_altp2m_op (-EFAULT if
> the copy didn't work or the result of hypercall_create_continuation
> otherwise).

And it's the result of hypercall_create_continuation() which you
want to check against (provided there's no other way for rc to
obtain a positive value).

Jan


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