On Thu, Mar 12, 2015 at 4:27 PM, Ian Campbell <ian.campb...@citrix.com> wrote:
> On Thu, 2015-03-12 at 14:52 +0000, Julien Grall wrote: > > On 12/03/15 14:13, Tamas K Lengyel wrote: > > > > > > > > > On Thu, Mar 12, 2015 at 2:50 PM, Julien Grall <julien.gr...@linaro.org > > > <mailto:julien.gr...@linaro.org>> wrote: > > > > > > Hi Tamas, > > > > > > On 06/03/15 21:24, Tamas K Lengyel wrote: > > > > +/* > > > > + * If mem_access is in use it might have been the reason why > get_page_from_gva > > > > + * failed to fetch the page, as it uses the MMU for the > permission checking. > > > > + * Only in these cases we do a software-based type check and > fetch the page if > > > > + * we indeed found a conflicting mem_access setting. > > > > + */ > > > > +static int check_type_get_page(vaddr_t gva, unsigned long flag, > > > > + struct page_info** page) > > > > +{ > > > > + long rc; > > > > + paddr_t ipa; > > > > + unsigned long maddr; > > > > + unsigned long mfn; > > > > + xenmem_access_t xma; > > > > + p2m_type_t t; > > > > + > > > > + rc = gva_to_ipa(gva, &ipa); > > > > > > I though a bit more about this call. > > > > > > gva_to_ipa only checks if the mapping has read-permission. That > would > > > allow a guest to write on read-only mapping. > > > > > > > > > You have to pass the flags to gva_to_ipa in order to avoid > > > re-introducing XSA-98 [1] > > > > > > > > > Here I really just care if the mapping exist to see if we have a > > > mem_access restriction, r/w permission checking is then performed > > > afterwards by checking the page type. If there are additional > > > restrictions on the page beside the type, those certainly should be > > > added. Can you point me to where that additional restriction is stored > > > so I can query for it? > > > > The R/W permission checking done afterwards is not enough. > > > > What does get_page_from_gva is: > > 1) Check the permission on Stage 1 page table (controlled by the > guest > > and translate VA -> IPA) > > 2) Check the permission on Stage 2 page table (controlled by the > > hypervisor and translate IPA -> PA). > > > > get_page_from_gva may fail because of 1), which is not related to > memaccess. > > > > Currently, check_type_get_page emulate only the check for 2). So you may > > end up to allow Xen writing in read-only mapping (from the Stage 1 POV). > > This was XSA-98. > > XSA-98 was purely about stage-2 permissions (e.g. read-only grants). The > fact that the resulting patch also checks stage-1 permissions is not a > security property AFAICT. > > Ian. > We could check stage-1 permissions by walking the guest page tables and looking at the pte permissions, however I'm not aware of having this function implemented within Xen. I have it implemented in LibVMI ( https://github.com/libvmi/libvmi/blob/master/libvmi/arch/arm_aarch32.c#L81) so I could potentially port it if necessary. However, since the in-guest permissions are in control of the guest to begin with, I don't see any added security benefit in doing so. Tamas
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
