Xen project Mailing List

On Thu, Mar 12, 2015 at 4:27 PM, Ian Campbell <ian.campbell@xxxxxxxxxx> 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.grall@xxxxxxxxxx
> > <mailto:julien.grall@xxxxxxxxxx>> 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

Re: [Xen-devel] [PATCH V13 3/7] xen/arm: Allow hypervisor access to mem_access protected pages