From: "Jan Beulich" Subject: [PATCH] NPT: temporarily retain page table mapping in do_recalc() Date: Mon, 05 May 2014 11:31:55 +0100 Message-ID: <536784BB020000780000EF15@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=__PartBF8D868B.0__=" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WhGBk-0007Vp-He for xen-devel@lists.xenproject.org; Mon, 05 May 2014 10:32:01 +0000 List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel Cc: Tim Deegan List-Id: xen-devel@lists.xenproject.org This is a MIME message. If you are reading this text, you may want to consider changing to a mail reader or gateway that understands how to properly handle MIME multipart messages. --=__PartBF8D868B.0__= Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Commit b3e024f3 ("x86/NPT: don't walk page tables when changing types on a range") neglected the fact that p2m_next_level() replaces the previous level's mapping with the new level's one, hence dereferencing a stale pointer the translation for which may no longer be available (timing dependent). Add a parameter to that function allowing the caller to request that the mapping be retained (the unmapping will be taken care of by the caller then). Signed-off-by: Jan Beulich --- a/xen/arch/x86/mm/p2m-pt.c +++ b/xen/arch/x86/mm/p2m-pt.c @@ -172,7 +172,7 @@ static void p2m_add_iommu_flags(l1_pgent static int p2m_next_level(struct p2m_domain *p2m, void **table, unsigned long *gfn_remainder, unsigned long gfn, u32 = shift, - u32 max, unsigned long type) + u32 max, unsigned long type, bool_t unmap) { l1_pgentry_t *l1_entry; l1_pgentry_t *p2m_entry; @@ -280,7 +280,8 @@ p2m_next_level(struct p2m_domain *p2m, v } =20 next =3D map_domain_page(l1e_get_pfn(*p2m_entry)); - unmap_domain_page(*table); + if ( unmap ) + unmap_domain_page(*table); *table =3D next; =20 return 0; @@ -319,7 +320,7 @@ static int p2m_pt_set_recalc_range(struc =20 err =3D p2m_next_level(p2m, &table, &gfn_remainder, first_gfn, i * PAGETABLE_ORDER, 1 << PAGETABLE_ORDER, - pgt[i - 1]); + pgt[i - 1], 1); if ( err ) goto out; } @@ -386,7 +387,7 @@ static int do_recalc(struct p2m_domain * =20 err =3D p2m_next_level(p2m, &table, &gfn_remainder, gfn, level * PAGETABLE_ORDER, 1 << PAGETABLE_ORDER= , - pgt[level - 1]); + pgt[level - 1], 0); if ( err ) goto out; =20 @@ -416,6 +417,7 @@ static int do_recalc(struct p2m_domain * clear_recalc(l1, e); p2m->write_p2m_entry(p2m, gfn, pent, e, level + 1); } + unmap_domain_page((void *)((unsigned long)pent & PAGE_MASK)); } =20 pent =3D p2m_find_entry(table, &gfn_remainder, gfn, @@ -519,7 +521,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m, table =3D map_domain_page(mfn_x(pagetable_get_mfn(p2m_get_pagetable(p2= m)))); rc =3D p2m_next_level(p2m, &table, &gfn_remainder, gfn, L4_PAGETABLE_SHIFT - PAGE_SHIFT, - L4_PAGETABLE_ENTRIES, PGT_l3_page_table); + L4_PAGETABLE_ENTRIES, PGT_l3_page_table, 1); if ( rc ) goto out; =20 @@ -565,7 +567,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m, { rc =3D p2m_next_level(p2m, &table, &gfn_remainder, gfn, L3_PAGETABLE_SHIFT - PAGE_SHIFT, - L3_PAGETABLE_ENTRIES, PGT_l2_page_table); + L3_PAGETABLE_ENTRIES, PGT_l2_page_table, 1); if ( rc ) goto out; } @@ -574,7 +576,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m, { rc =3D p2m_next_level(p2m, &table, &gfn_remainder, gfn, L2_PAGETABLE_SHIFT - PAGE_SHIFT, - L2_PAGETABLE_ENTRIES, PGT_l1_page_table); + L2_PAGETABLE_ENTRIES, PGT_l1_page_table, 1); if ( rc ) goto out; =20 --=__PartBF8D868B.0__= Content-Type: text/plain; name="NPT-recalc-keep-mapping.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="NPT-recalc-keep-mapping.patch" NPT: temporarily retain page table mapping in do_recalc()=0A=0ACommit = b3e024f3 ("x86/NPT: don't walk page tables when changing types=0Aon a = range") neglected the fact that p2m_next_level() replaces the=0Aprevious = level's mapping with the new level's one, hence dereferencing=0Aa stale = pointer the translation for which may no longer be available=0A(timing = dependent). Add a parameter to that function allowing the=0Acaller to = request that the mapping be retained (the unmapping will be=0Ataken care = of by the caller then).=0A=0ASigned-off-by: Jan Beulich = =0A=0A--- a/xen/arch/x86/mm/p2m-pt.c=0A+++ b/xen/arch/x86/mm/p2m-pt.c=0A@@ = -172,7 +172,7 @@ static void p2m_add_iommu_flags(l1_pgent=0A static int=0A = p2m_next_level(struct p2m_domain *p2m, void **table,=0A = unsigned long *gfn_remainder, unsigned long gfn, u32 shift,=0A- = u32 max, unsigned long type)=0A+ u32 max, unsigned long = type, bool_t unmap)=0A {=0A l1_pgentry_t *l1_entry;=0A l1_pgentry_t= *p2m_entry;=0A@@ -280,7 +280,8 @@ p2m_next_level(struct p2m_domain *p2m, = v=0A }=0A =0A next =3D map_domain_page(l1e_get_pfn(*p2m_entry));=0A= - unmap_domain_page(*table);=0A+ if ( unmap )=0A+ unmap_domain= _page(*table);=0A *table =3D next;=0A =0A return 0;=0A@@ -319,7 = +320,7 @@ static int p2m_pt_set_recalc_range(struc=0A =0A err =3D = p2m_next_level(p2m, &table, &gfn_remainder, first_gfn,=0A = i * PAGETABLE_ORDER, 1 << PAGETABLE_ORDER,=0A- = pgt[i - 1]);=0A+ pgt[i - 1], = 1);=0A if ( err )=0A goto out;=0A }=0A@@ -386,7 = +387,7 @@ static int do_recalc(struct p2m_domain *=0A =0A err =3D = p2m_next_level(p2m, &table, &gfn_remainder, gfn,=0A = level * PAGETABLE_ORDER, 1 << PAGETABLE_ORDER,=0A- = pgt[level - 1]);=0A+ pgt[level - 1], = 0);=0A if ( err )=0A goto out;=0A =0A@@ -416,6 +417,7 = @@ static int do_recalc(struct p2m_domain *=0A clear_recalc(l1,= e);=0A p2m->write_p2m_entry(p2m, gfn, pent, e, level + 1);=0A = }=0A+ unmap_domain_page((void *)((unsigned long)pent & = PAGE_MASK));=0A }=0A =0A pent =3D p2m_find_entry(table, &gfn_remain= der, gfn,=0A@@ -519,7 +521,7 @@ p2m_pt_set_entry(struct p2m_domain = *p2m,=0A table =3D map_domain_page(mfn_x(pagetable_get_mfn(p2m_get_page= table(p2m))));=0A rc =3D p2m_next_level(p2m, &table, &gfn_remainder, = gfn,=0A L4_PAGETABLE_SHIFT - PAGE_SHIFT,=0A- = L4_PAGETABLE_ENTRIES, PGT_l3_page_table);=0A+ = L4_PAGETABLE_ENTRIES, PGT_l3_page_table, 1);=0A if ( rc = )=0A goto out;=0A =0A@@ -565,7 +567,7 @@ p2m_pt_set_entry(struct = p2m_domain *p2m,=0A {=0A rc =3D p2m_next_level(p2m, &table, = &gfn_remainder, gfn,=0A L3_PAGETABLE_SHIFT - = PAGE_SHIFT,=0A- L3_PAGETABLE_ENTRIES, PGT_l2_pag= e_table);=0A+ L3_PAGETABLE_ENTRIES, PGT_l2_page_= table, 1);=0A if ( rc )=0A goto out;=0A }=0A@@ = -574,7 +576,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m,=0A {=0A = rc =3D p2m_next_level(p2m, &table, &gfn_remainder, gfn,=0A = L2_PAGETABLE_SHIFT - PAGE_SHIFT,=0A- = L2_PAGETABLE_ENTRIES, PGT_l1_page_table);=0A+ = L2_PAGETABLE_ENTRIES, PGT_l1_page_table, 1);=0A if ( rc )=0A = goto out;=0A =0A --=__PartBF8D868B.0__= Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --=__PartBF8D868B.0__=--