From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH V13 3/7] xen/arm: Allow hypervisor access to mem_access protected pages Date: Thu, 12 Mar 2015 14:52:33 +0000 Message-ID: <5501A831.6010009@linaro.org> References: <1425677073-13729-1-git-send-email-tklengyel@sec.in.tum.de> <1425677073-13729-4-git-send-email-tklengyel@sec.in.tum.de> <55019996.9050208@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tamas K Lengyel Cc: wei.liu2@citrix.com, Ian Campbell , Stefano Stabellini , Tim Deegan , Ian Jackson , xen-devel@lists.xen.org, stefano.stabellini@citrix.com, Jan Beulich , Keir Fraser List-Id: xen-devel@lists.xenproject.org On 12/03/15 14:13, Tamas K Lengyel wrote: > > > On Thu, Mar 12, 2015 at 2:50 PM, Julien Grall > 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. Regards, -- Julien Grall