Re: [Xen-devel] [PATCH v5] x86/hvm: Allow guest_request vm_events coming from userspace

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Tue, Aug 15, 2017 at 2:06 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> On 14.08.17 at 17:53, <tamas@xxxxxxxxxxxxx> wrote:
>> On Tue, Aug 8, 2017 at 2:27 AM, Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx> 
>> wrote:
>>> --- 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;
>>> +
>>
>> So the CPL check happens after the monitor check, which means this
>> will trigger regardless if the hypercall is coming from userspace or
>> kernelspace. Since the monitor option specifically says userspace,
>> this should probably get moved into the block where CPL was checked.
>
> What difference would this make? For CPL0 the hypercall is
> permitted anyway, and for CPL > 0 we specifically want to bypass
> the CPL check. Or are you saying you want to restrict the new
> check to just CPL3?
>

Yes, according to the name of this monitor option this should only
trigger a vm_event when the hypercall is coming from CPL3. However,
the way it is implemented right now I see that this monitor option
actually requires the other one to be enabled too. By itself this
monitor option will not work. So I would also like to ask that the
check in xen/common/monitor.c, if ( d->monitor.guest_request_enabled
), to be extended to be: if ( d->monitor.guest_request_enabled ||
d->monitor.guest_request_userspace_enabled )

Tamas

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