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

[Julien Grall <julien.grall@arm.com>]

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.

When XSM is not enabled (this is the default on Xen), XSM_TARGET allows 
the guest (see xsm_default_action) to call the operations. So I am not 
convince XSM is the right way to go.

>
>> Also, I have noticed that a guest is allowed to disable ALTP2M on ARM
>> because it set any param (x86 has some restriction on it). Similarly,
>> the ALTP2M parameter can be set multiple time.
>>
>
> Same here.

Give a look how x86 restrict the write to HVMOP_set_param.

Regards,

-- 
Julien Grall

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