Re: [Xen-devel] [PATCH 02/18] arm/altp2m: Add first altp2m HVMOP stubs.

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Wed, Jul 6, 2016 at 7:43 AM, Julien Grall <julien.grall@xxxxxxx> wrote:
> Hello Sergej,
>
>
> On 06/07/16 10:14, Sergej Proskurin wrote:
>>
>> On 07/05/2016 12:19 PM, Julien Grall wrote:
>>>
>>> Hello Sergej,
>>>
>>> On 04/07/16 12:45, Sergej Proskurin wrote:
>>>>
>>>> +static int do_altp2m_op(XEN_GUEST_HANDLE_PARAM(void) arg)
>>>> +{
>>>> +    struct xen_hvm_altp2m_op a;
>>>> +    struct domain *d = NULL;
>>>> +    int rc = 0;
>>>> +
>>>> +    if ( !hvm_altp2m_supported() )
>>>> +        return -EOPNOTSUPP;
>>>> +
>>>> +    if ( copy_from_guest(&a, arg, 1) )
>>>> +        return -EFAULT;
>>>> +
>>>> +    if ( a.pad1 || a.pad2 ||
>>>> +         (a.version != HVMOP_ALTP2M_INTERFACE_VERSION) ||
>>>> +         (a.cmd < HVMOP_altp2m_get_domain_state) ||
>>>> +         (a.cmd > HVMOP_altp2m_change_gfn) )
>>>> +        return -EINVAL;
>>>> +
>>>> +    d = (a.cmd != HVMOP_altp2m_vcpu_enable_notify) ?
>>>> +        rcu_lock_domain_by_any_id(a.domain) :
>>>> rcu_lock_current_domain();
>>>> +
>>>> +    if ( d == NULL )
>>>> +        return -ESRCH;
>>>> +
>>>> +    if ( (a.cmd != HVMOP_altp2m_get_domain_state) &&
>>>> +         (a.cmd != HVMOP_altp2m_set_domain_state) &&
>>>> +         !d->arch.altp2m_active )
>>>> +    {
>>>> +        rc = -EOPNOTSUPP;
>>>> +        goto out;
>>>> +    }
>>>> +
>>>> +    if ( (rc = xsm_hvm_altp2mhvm_op(XSM_TARGET, d)) )
>>>> +        goto out;
>>>
>>>
>>> I think this is the best place to ask a couple of questions related to
>>> who can access altp2m. Based on this call, a guest is allowed to
>>> manage its own altp2m. Can you explain why we would want a guest to do
>>> that?
>>>
>>
>> On x86, altp2m might be used by the guest in the #VE (Virtualization
>> Exception). On ARM, there is indeed not necessary for a guest to access
>> altp2m. Could you provide me with information, how to best restrict
>> non-privileged guests (not only dom0) from accessing these HVMOPs? Can
>> thisbedone by means of xsm? Thank you.
>
>
> This does not looks safe for both x86 and ARM. From my understanding a
> malware would be able to modify an altp2m, switching between 2 view... which
> would lead to remove the entire purpose of altp2m.

Well, the whole purpose of the VMFUNC instruction right now is to be
able to switch the hypervisor's tables (like altp2m) from within the
guest. So yes, if you have a malicious guest then it could control
it's own altp2m views. I would assume there are other safe-guards
in-place for systems that use this to ensure kernel-integrity and thus
prevent arbitrary use of these functions. AFAIK there are only
experimental systems based on this so whether it's less or more secure
is debatable and likely depend on your implementation.

As for ARM - as there is no hardware features like this available -
our goal is to use altp2m in purely external usecases so exposing
these ops to the guest is not required. For the first prototype it
made sense to mirror the x86 side to reduce the possibility of
introducing some bug.

Cheers,
Tamas

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