diff mbox

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

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

Commit Message

Alexandru Stefan ISAILA Aug. 1, 2017, 9:46 a.m. UTC
Allow guest userspace code to request that a vm_event be sent out
via VMCALL. This functionality seems to be handy for a number of
Xen developers, as stated on the mailing list (thread "[Xen-devel]
HVMOP_guest_request_vm_event only works from guest in ring0").
This is a use case in communication between a userspace application
in the guest and the introspection application in dom0.

Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>

---
Changes since V1:
	- Added Fallthrough check on mode == 2
---
 xen/arch/x86/hvm/hypercall.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Andrew Cooper Aug. 1, 2017, 10:30 a.m. UTC | #1
On 01/08/17 10:46, Alexandru Isaila wrote:
> Allow guest userspace code to request that a vm_event be sent out
> via VMCALL. This functionality seems to be handy for a number of
> Xen developers, as stated on the mailing list (thread "[Xen-devel]
> HVMOP_guest_request_vm_event only works from guest in ring0").
> This is a use case in communication between a userspace application
> in the guest and the introspection application in dom0.
>
> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>

This issue has been argued several times before, and while I am in
favour of the change, there is a legitimate argument that it breaks one
of our security boundaries.

One intermediate option comes to mind however.

Could we introduce a new monitor op which permits the use of
HVMOP_guest_request_vm_event from userspace?  This way, it requires a
positive action on behalf of the introspection agent to relax the CPL
check, rather than having the CPL check unconditionally relaxed.

I think this would be sufficient to alleviate the previous objections.

~Andrew

>
> ---
> Changes since V1:
> 	- Added Fallthrough check on mode == 2
> ---
>  xen/arch/x86/hvm/hypercall.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
> index e7238ce..1c067c3 100644
> --- a/xen/arch/x86/hvm/hypercall.c
> +++ b/xen/arch/x86/hvm/hypercall.c
> @@ -152,9 +152,15 @@ int hvm_hypercall(struct cpu_user_regs *regs)
>      {
>      case 8:
>          eax = regs->rax;
> +        if ( eax == __HYPERVISOR_hvm_op &&
> +             regs->rdi == HVMOP_guest_request_vm_event )
> +            break;
>          /* Fallthrough to permission check. */
>      case 4:
>      case 2:
> +        if ( mode != 8 && eax == __HYPERVISOR_hvm_op &&
> +             regs->ebx == HVMOP_guest_request_vm_event )
> +            break;
>          if ( unlikely(hvm_get_cpl(curr)) )
>          {
>      default:
Alexandru Stefan ISAILA Aug. 1, 2017, 11:01 a.m. UTC | #2
I'm sure we can to this and use a monitor op together with the 
HVMOP_guest_request_vm_event event. We have discussed this
and have a good idea on how to do it. 

~Alex
Tamas K Lengyel Aug. 1, 2017, 4:07 p.m. UTC | #3
On Tue, Aug 1, 2017 at 4:30 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> On 01/08/17 10:46, Alexandru Isaila wrote:
>> Allow guest userspace code to request that a vm_event be sent out
>> via VMCALL. This functionality seems to be handy for a number of
>> Xen developers, as stated on the mailing list (thread "[Xen-devel]
>> HVMOP_guest_request_vm_event only works from guest in ring0").
>> This is a use case in communication between a userspace application
>> in the guest and the introspection application in dom0.
>>
>> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
>
> This issue has been argued several times before, and while I am in
> favour of the change, there is a legitimate argument that it breaks one
> of our security boundaries.
>
> One intermediate option comes to mind however.
>
> Could we introduce a new monitor op which permits the use of
> HVMOP_guest_request_vm_event from userspace?  This way, it requires a
> positive action on behalf of the introspection agent to relax the CPL
> check, rather than having the CPL check unconditionally relaxed.

I agree, it would be required to gate this on a monitor option that is
disabled by default.

Tamas
Razvan Cojocaru Aug. 2, 2017, 1:32 p.m. UTC | #4
On 01.08.2017 19:07, Tamas K Lengyel wrote:
> On Tue, Aug 1, 2017 at 4:30 AM, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 01/08/17 10:46, Alexandru Isaila wrote:
>>> Allow guest userspace code to request that a vm_event be sent out
>>> via VMCALL. This functionality seems to be handy for a number of
>>> Xen developers, as stated on the mailing list (thread "[Xen-devel]
>>> HVMOP_guest_request_vm_event only works from guest in ring0").
>>> This is a use case in communication between a userspace application
>>> in the guest and the introspection application in dom0.
>>>
>>> Signed-off-by: Alexandru Isaila <aisaila@bitdefender.com>
>>
>> This issue has been argued several times before, and while I am in
>> favour of the change, there is a legitimate argument that it breaks one
>> of our security boundaries.
>>
>> One intermediate option comes to mind however.
>>
>> Could we introduce a new monitor op which permits the use of
>> HVMOP_guest_request_vm_event from userspace?  This way, it requires a
>> positive action on behalf of the introspection agent to relax the CPL
>> check, rather than having the CPL check unconditionally relaxed.
> 
> I agree, it would be required to gate this on a monitor option that is
> disabled by default.

That's the way we'll move forward. About that: would you prefer a new 
libxc function that toggles only the userspace part, or that we add a 
bool parameter to the existing xc_monitor_guest_request()?


Thanks,
Razvan
diff mbox

Patch

diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index e7238ce..1c067c3 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -152,9 +152,15 @@  int hvm_hypercall(struct cpu_user_regs *regs)
     {
     case 8:
         eax = regs->rax;
+        if ( eax == __HYPERVISOR_hvm_op &&
+             regs->rdi == HVMOP_guest_request_vm_event )
+            break;
         /* Fallthrough to permission check. */
     case 4:
     case 2:
+        if ( mode != 8 && eax == __HYPERVISOR_hvm_op &&
+             regs->ebx == HVMOP_guest_request_vm_event )
+            break;
         if ( unlikely(hvm_get_cpl(curr)) )
         {
     default: