From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v3 2/5] arch, arm: add consistency checks to REMOVE p2m changes Date: Fri, 21 Mar 2014 12:45:46 +0000 Message-ID: <532C347A.4020701@linaro.org> References: <1394914286-29713-1-git-send-email-avanzini.arianna@gmail.com> <1394914286-29713-3-git-send-email-avanzini.arianna@gmail.com> <1395398657.27358.27.camel@kazak.uk.xensource.com> <532C27B3.4050802@citrix.com> <1395402840.27358.66.camel@kazak.uk.xensource.com> <532C2BD1.7040800@linaro.org> <1395405162.19839.25.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1395405162.19839.25.camel@kazak.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: paolo.valente@unimore.it, keir@xen.org, stefano.stabellini@eu.citrix.com, Ian.Jackson@eu.citrix.com, dario.faggioli@citrix.com, tim@xen.org, xen-devel@lists.xen.org, etrudeau@broadcom.com, JBeulich@suse.com, Arianna Avanzini , viktor.kleinik@globallogic.com List-Id: xen-devel@lists.xenproject.org On 03/21/2014 12:32 PM, Ian Campbell wrote: >> The toolstack will only provide the gfn, except for this >> function. > > memory_unmap should also only take a gfn, which Xen should lookup to get > an mfn. Notice that on x86 the unmap case doesn't use the mfn argument > and passes only a gfn to clear_mmio_p2m_entry. The MFN argument is used to check if the domain has access to the MFN(see iomem_access_permitted at the beginning of the case). I think we should implement the behavior you described in the common implementation. > It's racy to have the toolstack provide it anyway. >>>> Otherwise the toolstack may be able to remove any page as long as the >>>> MFN is in the iomem permitted range. >>> >>> Can't it already do this through other paths? >>> >>> Maybe there is a security implication there, but I would hope that the >>> two permissions were pretty closely linked. >> >> One the main problem is iomem range permitted won't be anymore in sync. > > I don't think this is a big issue. Having permission to have a mapping > does not necessarily imply having the actual mapping, if the toolstack > wants to do it then let it. > >> x86 at least check that the gfn is an MMIO. I think we can safely extend >> to check that the GFN use the corresponding MFN. >> >> I don't agree to let the toolstack uses this DOMCTL to do remove any >> page in the guess memory. > > Well, it already can today AFAICS, via remove_from_physmap. remove_from_physmap can't remove MMIO region. There is a specific check in get_page_from_gfn for this purpose. IHMO, memory_unmap should avoid to remove other region than MMIO. Mainly because apply_p2m_changes might not remove every reference taken on a page. -- Julien Grall