[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 02/18] arm/altp2m: Add first altp2m HVMOP stubs.
(Add x86 maintainers)
On 06/07/16 16:23, Tamas K Lengyel wrote:
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.
Taken aside the VMFUNC, it looks like insecure to expose a HVMOP to the
guest which could modify the memaccess attribute of a region.
I thought the whole purpose of VM introspection is to avoid trusting the
guest (kernel + userspace). The first thing a malware will do is trying
to do is getting access to the kernel. Once it gets this access, it
could remove all the memory access restriction to avoid trapping.
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.
No, this is not the right approach. We should not introduce potential
security issue just because x86 side does it and it "reduces the
possibility of introducing some bug".
You will have to give me another reason to accept a such patch.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
