Message ID | 1503913126-13546-1-git-send-email-aisaila@bitdefender.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
>>> On 28.08.17 at 11:38, <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> For the parts it is applicable to: Acked-by: Jan Beulich <jbeulich@suse.com> I'd like to note though that I find it a little odd for &d->arch to be passed to a hook, instead of just d. But it'll be the maintainers of that code to approve (or not) of that. Jan
On Mon, Aug 28, 2017 at 12:38:46PM +0300, Alexandru Isaila 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>
> diff --git a/xen/common/monitor.c b/xen/common/monitor.c > index 451f42f..0c3e645 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_allow_userspace(&d->arch, mop->u.guest_request.allow_userspace); Please use the appropriate prefix with this function, ie. arch_monitor_allow_userspace. Thanks, Tamas
On Mon, Aug 28, 2017 at 5:10 AM, Jan Beulich <JBeulich@suse.com> wrote: >>>> On 28.08.17 at 11:38, <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> > > For the parts it is applicable to: > Acked-by: Jan Beulich <jbeulich@suse.com> > > I'd like to note though that I find it a little odd for &d->arch to be > passed to a hook, instead of just d. But it'll be the maintainers of > that code to approve (or not) of that. > Indeed, I don't see d->arch being passed like this anywhere else either. I don't think it breaks anything but for stylistic reasons it might be better to conform here too. Tamas
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..0c3e645 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_allow_userspace(&d->arch, 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..a2eec52 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_allow_userspace(struct arch_domain *arch, 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..a834d80 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_allow_userspace(struct arch_domain *arch, uint8_t allow_userspace) +{ + 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 {
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> --- Changes since V6: - Added arch specific function in both x86 monitor and arm monitor to replace the assignment from common monitor Note: Could not test on ARN, 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(-)