| Message ID | 1503998602-17326-1-git-send-email-aisaila@bitdefender.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 29.08.17 at 11:23, <aisaila@bitdefender.com> wrote: > 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> For the pieces it applies to Acked-by: Jan Beulich <jbeulich@suse.com> However, as I keep looking at pieces which shouldn't really require my attention, I've noticed one more cosmetic issue: > --- 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, bool allow_userspace) > +{ > + return; > +} I don't see the point of the return statement here. But I'll leave it to the ARM maintainers, and it would be easy to drop while committing if no other issues are going to arise. Jan
On Tue, Aug 29, 2017 at 3:36 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>> On 29.08.17 at 11:23, <aisaila@bitdefender.com> wrote: >> 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> > > For the pieces it applies to > Acked-by: Jan Beulich <jbeulich@suse.com> > > However, as I keep looking at pieces which shouldn't really require > my attention, I've noticed one more cosmetic issue: > >> --- 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, bool allow_userspace) >> +{ >> + return; >> +} > > I don't see the point of the return statement here. But I'll leave > it to the ARM maintainers, and it would be easy to drop while > committing if no other issues are going to arise. > I would bet that a sane compiler wouldn't generate anything for that with or without return in there but sure, if we can drop it then why not. Tamas
On Tue, Aug 29, 2017 at 3:23 AM, Alexandru Isaila <aisaila@bitdefender.com> wrote: > 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> Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
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..2bdad7d 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, bool 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..765d0b4 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, bool 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 {