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

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

[Razvan Cojocaru]

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").

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

---
Changes since V1:
 - No longer repeating the check when mode == 8.
 - Added /* Fallthrough */ tags for Coverity.
---
 xen/arch/x86/hvm/hvm.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 893eff6..cb546e4 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4146,15 +4146,25 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
     switch ( mode )
     {
     case 8:        
+        if ( eax == __HYPERVISOR_hvm_op &&
+             regs->rdi == HVMOP_guest_request_vm_event )
+            break;
+        /* Fallthrough */
     case 4:
+        /* Fallthrough */
     case 2:
+        if ( mode != 8 && eax == __HYPERVISOR_hvm_op &&
+             regs->_ebx == HVMOP_guest_request_vm_event )
+            break;
         hvm_get_segment_register(curr, x86_seg_ss, &sreg);
         if ( unlikely(sreg.attr.fields.dpl) )
         {
+        /* Fallthrough */
     default:
             regs->eax = -EPERM;
             return HVM_HCALL_completed;
         }
+        /* Fallthrough */
     case 0:
         break;
     }
-- 
1.9.1


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

Re: [PATCH V2] x86/hvm: Allow guest_request vm_events coming from userspace

[Jan Beulich]

>>> On 08.08.16 at 10:06, <rcojocaru@bitdefender.com> 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").
> 
> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
> 
> ---
> Changes since V1:
>  - No longer repeating the check when mode == 8.

Technically this looks correct to me now. Albeit I'm still not really
convinced we actually want to start making exceptions here.

> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4146,15 +4146,25 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
>      switch ( mode )
>      {
>      case 8:        
> +        if ( eax == __HYPERVISOR_hvm_op &&
> +             regs->rdi == HVMOP_guest_request_vm_event )
> +            break;
> +        /* Fallthrough */
>      case 4:
> +        /* Fallthrough */
>      case 2:

At least this one annotation is pointless, but if we decide to commit
the change this can of course be taken care of while committing.

> +        if ( mode != 8 && eax == __HYPERVISOR_hvm_op &&
> +             regs->_ebx == HVMOP_guest_request_vm_event )
> +            break;
>          hvm_get_segment_register(curr, x86_seg_ss, &sreg);
>          if ( unlikely(sreg.attr.fields.dpl) )
>          {
> +        /* Fallthrough */
>      default:

I would hope this annotation to be pointless too, but that would
need to be clarified by someone more familiar with Coverity.

>              regs->eax = -EPERM;
>              return HVM_HCALL_completed;
>          }
> +        /* Fallthrough */
>      case 0:

This one, otoh, looks like it was indeed missing (and Coverity
should have complained).

Jan


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

Re: [PATCH V2] x86/hvm: Allow guest_request vm_events coming from userspace

[Razvan Cojocaru]

On 08/08/2016 02:34 PM, Jan Beulich wrote:
>>>> On 08.08.16 at 10:06, <rcojocaru@bitdefender.com> 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").
>>
>> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
>>
>> ---
>> Changes since V1:
>>  - No longer repeating the check when mode == 8.
> 
> Technically this looks correct to me now. Albeit I'm still not really
> convinced we actually want to start making exceptions here.

I am in any case happy that it's been properly reviewed (thank you!).

The exception here is IMO warranted by the fact that this particular
event has been especially designed so as to be able to signal from the
guest to an introspection application, hence the special status.
Considering also that, as Andrew has pointed out, alternatives are
possible, and that there are several interested users, I thought it
would be worth a shot.

But in the end it is, understandably, up to the maintainers.


Thanks again,
Razvan

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

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

[Razvan Cojocaru]

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

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

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

[Tamas K Lengyel]

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

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

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

[Alexandru Stefan ISAILA]

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
________________________________________
From: Andrew Cooper <andrew.cooper3@citrix.com>
Sent: Tuesday, August 1, 2017 1:30 PM
To: Alexandru Stefan ISAILA; xen-devel@lists.xen.org
Cc: jbeulich@suse.com; Razvan Cojocaru; Tamas K Lengyel
Subject: Re: [PATCH v2] x86/hvm: Allow guest_request vm_events coming from userspace

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:


________________________
This email was scanned by Bitdefender

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

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

[Andrew Cooper]

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:


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

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

[Alexandru Isaila]

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(+)

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:
-- 
2.7.4


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