* [PATCH] x86/p2m-pt: delay freeing of intermediate page tables
@ 2015-09-29 10:51 Jan Beulich
2015-09-29 10:54 ` George Dunlap
2015-09-29 12:33 ` Wei Liu
0 siblings, 2 replies; 3+ messages in thread
From: Jan Beulich @ 2015-09-29 10:51 UTC (permalink / raw)
To: xen-devel; +Cc: George Dunlap, Andrew Cooper, Wei Liu
[-- Attachment #1: Type: text/plain, Size: 3662 bytes --]
Old intermediate page tables must be freed only after IOMMU side
updates/flushes have got carried out.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
This was split out from a previously submitted patch.
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -487,8 +487,9 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
/* XXX -- this might be able to be faster iff current->domain == d */
void *table;
unsigned long i, gfn_remainder = gfn;
- l1_pgentry_t *p2m_entry;
- l1_pgentry_t entry_content;
+ l1_pgentry_t *p2m_entry, entry_content;
+ /* Intermediate table to free if we're replacing it with a superpage. */
+ l1_pgentry_t intermediate_entry = l1e_empty();
l2_pgentry_t l2e_content;
l3_pgentry_t l3e_content;
int rc;
@@ -538,7 +539,6 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
*/
if ( page_order == PAGE_ORDER_1G )
{
- l1_pgentry_t old_entry = l1e_empty();
p2m_entry = p2m_find_entry(table, &gfn_remainder, gfn,
L3_PAGETABLE_SHIFT - PAGE_SHIFT,
L3_PAGETABLE_ENTRIES);
@@ -548,7 +548,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
{
/* We're replacing a non-SP page with a superpage. Make sure to
* handle freeing the table properly. */
- old_entry = *p2m_entry;
+ intermediate_entry = *p2m_entry;
}
ASSERT(!mfn_valid(mfn) || p2mt != p2m_mmio_direct);
@@ -566,10 +566,6 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
p2m->write_p2m_entry(p2m, gfn, p2m_entry, entry_content, 3);
/* NB: paging_write_p2m_entry() handles tlb flushes properly */
-
- /* Free old intermediate tables if necessary */
- if ( l1e_get_flags(old_entry) & _PAGE_PRESENT )
- p2m_free_entry(p2m, &old_entry, page_order);
}
else
{
@@ -610,7 +606,6 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
}
else if ( page_order == PAGE_ORDER_2M )
{
- l1_pgentry_t old_entry = l1e_empty();
p2m_entry = p2m_find_entry(table, &gfn_remainder, gfn,
L2_PAGETABLE_SHIFT - PAGE_SHIFT,
L2_PAGETABLE_ENTRIES);
@@ -622,7 +617,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
{
/* We're replacing a non-SP page with a superpage. Make sure to
* handle freeing the table properly. */
- old_entry = *p2m_entry;
+ intermediate_entry = *p2m_entry;
}
ASSERT(!mfn_valid(mfn) || p2mt != p2m_mmio_direct);
@@ -643,10 +638,6 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
p2m->write_p2m_entry(p2m, gfn, p2m_entry, entry_content, 2);
/* NB: paging_write_p2m_entry() handles tlb flushes properly */
-
- /* Free old intermediate tables if necessary */
- if ( l1e_get_flags(old_entry) & _PAGE_PRESENT )
- p2m_free_entry(p2m, &old_entry, page_order);
}
/* Track the highest gfn for which we have ever had a valid mapping */
@@ -674,6 +665,14 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
}
}
+ /*
+ * Free old intermediate tables if necessary. This has to be the
+ * last thing we do, after removal from the IOMMU tables, so as to
+ * avoid a potential use-after-free.
+ */
+ if ( l1e_get_flags(intermediate_entry) & _PAGE_PRESENT )
+ p2m_free_entry(p2m, &intermediate_entry, page_order);
+
out:
unmap_domain_page(table);
return rc;
[-- Attachment #2: x86-p2m-pt-delay-free.patch --]
[-- Type: text/plain, Size: 3713 bytes --]
x86/p2m-pt: delay freeing of intermediate page tables
Old intermediate page tables must be freed only after IOMMU side
updates/flushes have got carried out.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
This was split out from a previously submitted patch.
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -487,8 +487,9 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
/* XXX -- this might be able to be faster iff current->domain == d */
void *table;
unsigned long i, gfn_remainder = gfn;
- l1_pgentry_t *p2m_entry;
- l1_pgentry_t entry_content;
+ l1_pgentry_t *p2m_entry, entry_content;
+ /* Intermediate table to free if we're replacing it with a superpage. */
+ l1_pgentry_t intermediate_entry = l1e_empty();
l2_pgentry_t l2e_content;
l3_pgentry_t l3e_content;
int rc;
@@ -538,7 +539,6 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
*/
if ( page_order == PAGE_ORDER_1G )
{
- l1_pgentry_t old_entry = l1e_empty();
p2m_entry = p2m_find_entry(table, &gfn_remainder, gfn,
L3_PAGETABLE_SHIFT - PAGE_SHIFT,
L3_PAGETABLE_ENTRIES);
@@ -548,7 +548,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
{
/* We're replacing a non-SP page with a superpage. Make sure to
* handle freeing the table properly. */
- old_entry = *p2m_entry;
+ intermediate_entry = *p2m_entry;
}
ASSERT(!mfn_valid(mfn) || p2mt != p2m_mmio_direct);
@@ -566,10 +566,6 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
p2m->write_p2m_entry(p2m, gfn, p2m_entry, entry_content, 3);
/* NB: paging_write_p2m_entry() handles tlb flushes properly */
-
- /* Free old intermediate tables if necessary */
- if ( l1e_get_flags(old_entry) & _PAGE_PRESENT )
- p2m_free_entry(p2m, &old_entry, page_order);
}
else
{
@@ -610,7 +606,6 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
}
else if ( page_order == PAGE_ORDER_2M )
{
- l1_pgentry_t old_entry = l1e_empty();
p2m_entry = p2m_find_entry(table, &gfn_remainder, gfn,
L2_PAGETABLE_SHIFT - PAGE_SHIFT,
L2_PAGETABLE_ENTRIES);
@@ -622,7 +617,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
{
/* We're replacing a non-SP page with a superpage. Make sure to
* handle freeing the table properly. */
- old_entry = *p2m_entry;
+ intermediate_entry = *p2m_entry;
}
ASSERT(!mfn_valid(mfn) || p2mt != p2m_mmio_direct);
@@ -643,10 +638,6 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
p2m->write_p2m_entry(p2m, gfn, p2m_entry, entry_content, 2);
/* NB: paging_write_p2m_entry() handles tlb flushes properly */
-
- /* Free old intermediate tables if necessary */
- if ( l1e_get_flags(old_entry) & _PAGE_PRESENT )
- p2m_free_entry(p2m, &old_entry, page_order);
}
/* Track the highest gfn for which we have ever had a valid mapping */
@@ -674,6 +665,14 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
}
}
+ /*
+ * Free old intermediate tables if necessary. This has to be the
+ * last thing we do, after removal from the IOMMU tables, so as to
+ * avoid a potential use-after-free.
+ */
+ if ( l1e_get_flags(intermediate_entry) & _PAGE_PRESENT )
+ p2m_free_entry(p2m, &intermediate_entry, page_order);
+
out:
unmap_domain_page(table);
return rc;
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] x86/p2m-pt: delay freeing of intermediate page tables
2015-09-29 10:51 [PATCH] x86/p2m-pt: delay freeing of intermediate page tables Jan Beulich
@ 2015-09-29 10:54 ` George Dunlap
2015-09-29 12:33 ` Wei Liu
1 sibling, 0 replies; 3+ messages in thread
From: George Dunlap @ 2015-09-29 10:54 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: George Dunlap, Andrew Cooper, Wei Liu
On 29/09/15 11:51, Jan Beulich wrote:
> Old intermediate page tables must be freed only after IOMMU side
> updates/flushes have got carried out.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
> ---
> This was split out from a previously submitted patch.
>
> --- a/xen/arch/x86/mm/p2m-pt.c
> +++ b/xen/arch/x86/mm/p2m-pt.c
> @@ -487,8 +487,9 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
> /* XXX -- this might be able to be faster iff current->domain == d */
> void *table;
> unsigned long i, gfn_remainder = gfn;
> - l1_pgentry_t *p2m_entry;
> - l1_pgentry_t entry_content;
> + l1_pgentry_t *p2m_entry, entry_content;
> + /* Intermediate table to free if we're replacing it with a superpage. */
> + l1_pgentry_t intermediate_entry = l1e_empty();
> l2_pgentry_t l2e_content;
> l3_pgentry_t l3e_content;
> int rc;
> @@ -538,7 +539,6 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
> */
> if ( page_order == PAGE_ORDER_1G )
> {
> - l1_pgentry_t old_entry = l1e_empty();
> p2m_entry = p2m_find_entry(table, &gfn_remainder, gfn,
> L3_PAGETABLE_SHIFT - PAGE_SHIFT,
> L3_PAGETABLE_ENTRIES);
> @@ -548,7 +548,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
> {
> /* We're replacing a non-SP page with a superpage. Make sure to
> * handle freeing the table properly. */
> - old_entry = *p2m_entry;
> + intermediate_entry = *p2m_entry;
> }
>
> ASSERT(!mfn_valid(mfn) || p2mt != p2m_mmio_direct);
> @@ -566,10 +566,6 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
>
> p2m->write_p2m_entry(p2m, gfn, p2m_entry, entry_content, 3);
> /* NB: paging_write_p2m_entry() handles tlb flushes properly */
> -
> - /* Free old intermediate tables if necessary */
> - if ( l1e_get_flags(old_entry) & _PAGE_PRESENT )
> - p2m_free_entry(p2m, &old_entry, page_order);
> }
> else
> {
> @@ -610,7 +606,6 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
> }
> else if ( page_order == PAGE_ORDER_2M )
> {
> - l1_pgentry_t old_entry = l1e_empty();
> p2m_entry = p2m_find_entry(table, &gfn_remainder, gfn,
> L2_PAGETABLE_SHIFT - PAGE_SHIFT,
> L2_PAGETABLE_ENTRIES);
> @@ -622,7 +617,7 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
> {
> /* We're replacing a non-SP page with a superpage. Make sure to
> * handle freeing the table properly. */
> - old_entry = *p2m_entry;
> + intermediate_entry = *p2m_entry;
> }
>
> ASSERT(!mfn_valid(mfn) || p2mt != p2m_mmio_direct);
> @@ -643,10 +638,6 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
>
> p2m->write_p2m_entry(p2m, gfn, p2m_entry, entry_content, 2);
> /* NB: paging_write_p2m_entry() handles tlb flushes properly */
> -
> - /* Free old intermediate tables if necessary */
> - if ( l1e_get_flags(old_entry) & _PAGE_PRESENT )
> - p2m_free_entry(p2m, &old_entry, page_order);
> }
>
> /* Track the highest gfn for which we have ever had a valid mapping */
> @@ -674,6 +665,14 @@ p2m_pt_set_entry(struct p2m_domain *p2m,
> }
> }
>
> + /*
> + * Free old intermediate tables if necessary. This has to be the
> + * last thing we do, after removal from the IOMMU tables, so as to
> + * avoid a potential use-after-free.
> + */
> + if ( l1e_get_flags(intermediate_entry) & _PAGE_PRESENT )
> + p2m_free_entry(p2m, &intermediate_entry, page_order);
> +
> out:
> unmap_domain_page(table);
> return rc;
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] x86/p2m-pt: delay freeing of intermediate page tables
2015-09-29 10:51 [PATCH] x86/p2m-pt: delay freeing of intermediate page tables Jan Beulich
2015-09-29 10:54 ` George Dunlap
@ 2015-09-29 12:33 ` Wei Liu
1 sibling, 0 replies; 3+ messages in thread
From: Wei Liu @ 2015-09-29 12:33 UTC (permalink / raw)
To: Jan Beulich; +Cc: George Dunlap, xen-devel, Wei Liu, Andrew Cooper
On Tue, Sep 29, 2015 at 04:51:46AM -0600, Jan Beulich wrote:
> Old intermediate page tables must be freed only after IOMMU side
> updates/flushes have got carried out.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-09-29 12:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-29 10:51 [PATCH] x86/p2m-pt: delay freeing of intermediate page tables Jan Beulich
2015-09-29 10:54 ` George Dunlap
2015-09-29 12:33 ` Wei Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).