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

[Julien Grall <julien.grall@xxxxxxx>]

On 06/07/16 17:05, Tamas K Lengyel wrote:
> On Wed, Jul 6, 2016 at 9:54 AM, Julien Grall <julien.grall@xxxxxxx> wrote:
> > 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.
> That's why I'm saying systems that use this will likely do extra steps
> to ensure kernel integrity. In use-cases where this is to be used
> exclusively for external monitoring the system can be restricted with
> XSM to not allow the guest to issue the hvmops. And remember, on x86
> this system is not exclusively used for introspection.
I am not aware on how x86 is using alt2pm. And this series didn't give 
much explanation how this is supposed to work...
> > > 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.
> The first revision of a large series is highly unlikely to get
> accepted on the first run so we have been working with the assumption
> that there will be new revisions. The prototype has been working well
> enough for our internal tests to warrant not submitting it as PATCH
> RFC. Since this is Sergej's first work with Xen it helped to mirror
> the x86 to get him up to speed while working on the prototype and
> reducing the complexity he has to keep track of. Now that this phase
> is complete the adjustments can be made as required, such as not
> exposing these hvmops to ARM guests.
A such large series is already hard to review, it is even harder when 
the contributor leaves code unfinished because he assumed there will be 
a new revision. Actually this is the whole purpose of tagging the series 
with "RFC". It is used that the series is not in a state where it could 
potentially be committed.
Regards,

--
Julien Grall

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