Re: [Xen-devel] Xen ARM - Exposing a PL011 to the guest

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

Sorry for the late answer, I am just back from holidays and still 
catching-up with my e-mails.
On 03/01/17 20:08, Stefano Stabellini wrote:
> On Thu, 29 Dec 2016, Bhupinder Thakur wrote:
> > On 28 December 2016 at 23:19, Julien Grall <julien.grall@xxxxxxx> wrote:
> > > On 21/12/16 22:12, Stefano Stabellini wrote:
> > > > On Wed, 21 Dec 2016, Julien Grall wrote:
> > > > > On 20/12/2016 20:53, Stefano Stabellini wrote:
> > > > > > On Tue, 20 Dec 2016, Julien Grall wrote:
> > > > > > > On 19/12/2016 21:24, Stefano Stabellini wrote:
> > > > > > > > On Mon, 19 Dec 2016, Christoffer Dall wrote:
> > > > > > > > > On Fri, Dec 16, 2016 at 05:03:13PM +0000, Julien Grall wrote:
> > > > > > > > If we use hvm_params for this, we need two new hvm_params and Xen
> > > > > > > > needs
> > > > > > > > to unmap the pfn from the guest immediately, because we don't want the
> > > > > > > > guest to have access to it.
> > > > > > >
> > > > > > > If you unmap the pfn, the PV backend will not be able to request the
> > > > > > > page
> > > > > > > because there will be no translation available.
> > > > > > >
> > > > > > > So what you want to do is preventing the guest to at least write into
> > > > > > > region
> > > > > > > (not sure if it is worth to restrict read)
> > > > > >
> > > > > > That's a good idea.
> > > > > >
> > > > > >
> > > > > > > and unmap the page via the hypercall XENMEM_decrease_reservation.
> > > > > >
> > > > > > That would be issued by the guest itself, right? To save address space?
> > > > >
> > > > > Correct. The main use case today is ballooning, but guest could call it
> > > > > on any
> > > > > other RAM baked page.
> > > > >
> > > > > I was thinking about more about the protection needed. Technically the
> > > > > data in
> > > > > the ring are not trusted. So if the guest is messing up with it, it would
> > > > > not
> > > > > be a big issue. Or did I miss anything here?
> > > >
> > > > I understand that a guest would be smart to call
> > > > XENMEM_decrease_reservation on the PV console page for pl011, but it
> > > > cannot be a security measure, because, in fact, it needs to be called by
> > > > the guest.  Of course, a malicious guest can simply not call
> > > > XENMEM_decrease_reservation for it.
> > >
> > > Sorry I was not clear. I was not suggested the guest to call
> > > XENMEM_decrease_reservation on ring for security but a malicious guest
> > > issuing the hypercall on the ring protected and replacing by another page.
> > >
> > > This is the exact same problem as the one I mentioned on the ITS thread. The
> > > page live in guest memory but contains data that will only be touched by
> > > Xen.
> > >
> > > If you remove those page from stage-2, the translation IPA -> MFN will be
> > > lost unless you store somewhere else. You would have to do it per-page as
> > > the buffer will use contiguous IPA but potentially noncontiguous MFN.
> > >
> > > In the case of ITS the memory is provisioned by the guest. So there are not
> > > much to do there except adding protection in stage-2 such as write
> > > protection and preventing the guest to unmap it. However for the pl011 ring,
> > > as Andrew pointed on IRC, what we need to do is accounting this page to the
> > > domain memory. No mapping is necessary in stage-2.
> > Please clarify what is meant by that no stage-2 mapping is required.
> > Does it mean that no stage-2 mapping is required for the guest as it
> > never needs to access this page?
> That's right.
>
>
> > However, the Xen HYP will need the stage-2 mapping to find out the
> > pl011 PFN --> physical MFN mapping so that it can map the page to its
> > own address space. Currently, I am using prepare_ring_for_helper () to
> > map the pl011 PFN (passed via hvm call) ---> phyiscal MFN ---> Xen HYP
> > VA.
> I am not sure what Julien had in mind exactly. I like the idea of not
> mapping the page at stage-2, but it is true that many interfaces expect
> pfns. If Xen is the one to allocate the pl011 PV console page, then Xen
> knows the mfn and could use it to map the page, instead of the pfn.
> However, the PV console backend also needs to map the same page, and it
> currently does that by calling xc_map_foreign_range, which I believe
> also expect a pfn.
Do you agree that page such as ioreq and the pl011 PV console are only 
used for communication between Xen and a backend? We don't want the 
guest to access the content of the pages but still be able to map. I 
would guess that using the gfn was a convenience but has some security 
impact (see  guess using the gfn is mostly for convenience but has some 
security impact today (see XSA-197 and XSA-199). AFAIU, we also want to 
account those pages to a domain.
For the credit, this is a suggestion made by Andrew on IRC. If I 
remember correctly what he suggested, it would be to have an 
add-to-physmap version restricted for non-domU components (e.g backend, 
toolstack) which would allow to map a given page (maybe via an index?).
> So maybe it is easier to use write-protection in stage-2 (as for ITS),
> unless Julien has a better idea?
We need write-protection and also prevent the page to be removed.

But to be fair, I would not bother to try to write-protect it for now if 
we are going towards keeping the pl011 console in the guest memory. I 
don't think letting the guest writing in the console page is a big 
problem. The backend console should already be protected against 
malicious guest. If not, we are already in trouble as it part of the PV 
protocol :).
Regarding Xen, the console protocol is very easy and I think only bound 
checking is sufficient. So what a guest could do is hijacking its own 
console. Any thoughts?
Cheers,

--
Julien Grall

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