diff mbox

[v9] x86/hvm: Allow guest_request vm_events coming from userspace

Message ID 1503994166-4552-1-git-send-email-aisaila@bitdefender.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alexandru Stefan ISAILA Aug. 29, 2017, 8:09 a.m. UTC 
In some introspection usecases, an in-guest agent needs to communicate
with the external introspection agent.  An existing mechanism is
HVMOP_guest_request_vm_event, but this is restricted to kernel usecases
like all other hypercalls.

Introduce a mechanism whereby the introspection agent can whitelist the
use of HVMOP_guest_request_vm_event directly from userspace.

Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>

---
Changes since V8:
	- Changed funtion name from arch_allow_userspace to
	arch_monitor_allow_userspace
	- Added Wei Liu's ack from v7

Note: Could not test on ARM, compiled both on arm and x86
---
 tools/libxc/include/xenctrl.h |  2 +-
 tools/libxc/xc_monitor.c      |  3 ++-
 xen/arch/x86/hvm/hypercall.c  |  5 +++++
 xen/common/monitor.c          |  1 +
 xen/include/asm-arm/monitor.h |  6 ++++++
 xen/include/asm-x86/domain.h  | 19 ++++++++++---------
 xen/include/asm-x86/monitor.h |  6 ++++++
 xen/include/public/domctl.h   |  1 +
 8 files changed, 32 insertions(+), 11 deletions(-)

Comments

Jan Beulich Aug. 29, 2017, 8:53 a.m. UTC | #1 
>>> On 29.08.17 at 10:09, <aisaila@bitdefender.com> wrote:
> +        unsigned int guest_request_userspace_enabled                       : 1;

This being a single bit, ...

> --- a/xen/include/asm-x86/monitor.h
> +++ b/xen/include/asm-x86/monitor.h
> @@ -33,6 +33,12 @@ struct monitor_msr_bitmap {
>  };
>  
>  static inline
> +void arch_monitor_allow_userspace(struct domain *d, uint8_t allow_userspace)

... I think you want to use bool here, or else ...

> +{
> +    d->arch.monitor.guest_request_userspace_enabled = allow_userspace;

... assigning the 8-bit value here would truncate the high 7 bits
instead of the presumably intended meaning of non-zero -> true
and zero -> false in ...

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -1124,6 +1124,7 @@ struct xen_domctl_monitor_op {
>          struct {
>              /* Pause vCPU until response */
>              uint8_t sync;
> +            uint8_t allow_userspace;
>          } guest_request;

... the public interface.

Jan
diff mbox

Patch

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index bde8313..a3d0929 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2021,7 +2021,7 @@  int xc_monitor_software_breakpoint(xc_interface *xch, domid_t domain_id,
 int xc_monitor_descriptor_access(xc_interface *xch, domid_t domain_id,
                                  bool enable);
 int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id,
-                             bool enable, bool sync);
+                             bool enable, bool sync, bool allow_userspace);
 int xc_monitor_debug_exceptions(xc_interface *xch, domid_t domain_id,
                                 bool enable, bool sync);
 int xc_monitor_cpuid(xc_interface *xch, domid_t domain_id, bool enable);
diff --git a/tools/libxc/xc_monitor.c b/tools/libxc/xc_monitor.c
index b44ce93..a677820 100644
--- a/tools/libxc/xc_monitor.c
+++ b/tools/libxc/xc_monitor.c
@@ -147,7 +147,7 @@  int xc_monitor_descriptor_access(xc_interface *xch, domid_t domain_id,
 }
 
 int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id, bool enable,
-                             bool sync)
+                             bool sync, bool allow_userspace)
 {
     DECLARE_DOMCTL;
 
@@ -157,6 +157,7 @@  int xc_monitor_guest_request(xc_interface *xch, domid_t domain_id, bool enable,
                                     : XEN_DOMCTL_MONITOR_OP_DISABLE;
     domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_GUEST_REQUEST;
     domctl.u.monitor_op.u.guest_request.sync = sync;
+    domctl.u.monitor_op.u.guest_request.allow_userspace = enable ? allow_userspace : false;
 
     return do_domctl(xch, &domctl);
 }
diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index e7238ce..5742dd1 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -155,6 +155,11 @@  int hvm_hypercall(struct cpu_user_regs *regs)
         /* Fallthrough to permission check. */
     case 4:
     case 2:
+        if ( currd->arch.monitor.guest_request_userspace_enabled &&
+            eax == __HYPERVISOR_hvm_op &&
+            (mode == 8 ? regs->rdi : regs->ebx) == HVMOP_guest_request_vm_event )
+            break;
+
         if ( unlikely(hvm_get_cpl(curr)) )
         {
     default:
diff --git a/xen/common/monitor.c b/xen/common/monitor.c
index 451f42f..4c540e5 100644
--- a/xen/common/monitor.c
+++ b/xen/common/monitor.c
@@ -75,6 +75,7 @@  int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op *mop)
         domain_pause(d);
         d->monitor.guest_request_sync = mop->u.guest_request.sync;
         d->monitor.guest_request_enabled = requested_status;
+        arch_monitor_allow_userspace(d, mop->u.guest_request.allow_userspace);
         domain_unpause(d);
         break;
     }
diff --git a/xen/include/asm-arm/monitor.h b/xen/include/asm-arm/monitor.h
index 1c4fea3..e9dbcdb 100644
--- a/xen/include/asm-arm/monitor.h
+++ b/xen/include/asm-arm/monitor.h
@@ -26,6 +26,12 @@ 
 #include <public/domctl.h>
 
 static inline
+void arch_monitor_allow_userspace(struct domain *d, uint8_t allow_userspace)
+{
+    return;
+}
+
+static inline
 int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
 {
     /* No arch-specific monitor ops on ARM. */
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index c10522b..de02507 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -396,15 +396,16 @@  struct arch_domain
 
     /* Arch-specific monitor options */
     struct {
-        unsigned int write_ctrlreg_enabled       : 4;
-        unsigned int write_ctrlreg_sync          : 4;
-        unsigned int write_ctrlreg_onchangeonly  : 4;
-        unsigned int singlestep_enabled          : 1;
-        unsigned int software_breakpoint_enabled : 1;
-        unsigned int debug_exception_enabled     : 1;
-        unsigned int debug_exception_sync        : 1;
-        unsigned int cpuid_enabled               : 1;
-        unsigned int descriptor_access_enabled   : 1;
+        unsigned int write_ctrlreg_enabled                                 : 4;
+        unsigned int write_ctrlreg_sync                                    : 4;
+        unsigned int write_ctrlreg_onchangeonly                            : 4;
+        unsigned int singlestep_enabled                                    : 1;
+        unsigned int software_breakpoint_enabled                           : 1;
+        unsigned int debug_exception_enabled                               : 1;
+        unsigned int debug_exception_sync                                  : 1;
+        unsigned int cpuid_enabled                                         : 1;
+        unsigned int descriptor_access_enabled                             : 1;
+        unsigned int guest_request_userspace_enabled                       : 1;
         struct monitor_msr_bitmap *msr_bitmap;
         uint64_t write_ctrlreg_mask[4];
     } monitor;
diff --git a/xen/include/asm-x86/monitor.h b/xen/include/asm-x86/monitor.h
index c5c323b..54ec6d6 100644
--- a/xen/include/asm-x86/monitor.h
+++ b/xen/include/asm-x86/monitor.h
@@ -33,6 +33,12 @@  struct monitor_msr_bitmap {
 };
 
 static inline
+void arch_monitor_allow_userspace(struct domain *d, uint8_t allow_userspace)
+{
+    d->arch.monitor.guest_request_userspace_enabled = allow_userspace;
+}
+
+static inline
 int arch_monitor_domctl_op(struct domain *d, struct xen_domctl_monitor_op *mop)
 {
     int rc = 0;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index ff39762..5997c52 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -1124,6 +1124,7 @@  struct xen_domctl_monitor_op {
         struct {
             /* Pause vCPU until response */
             uint8_t sync;
+            uint8_t allow_userspace;
         } guest_request;
 
         struct {