Discussion:
[Xen-changelog] [xen staging] x86: correct instances of PGC_allocated clearing
p***@xen.org
2018-11-21 10:00:35 UTC
Permalink
commit 0502e0adae25f8d9e5b3f02320aa2859041f205d
Author: Jan Beulich <***@suse.com>
AuthorDate: Wed Nov 21 10:54:05 2018 +0100
Commit: Jan Beulich <***@suse.com>
CommitDate: Wed Nov 21 10:54:05 2018 +0100

x86: correct instances of PGC_allocated clearing

For domain heap pages assigned to a domain dropping the page reference
tied to PGC_allocated may not drop the last reference, as otherwise the
test_and_clear_bit() might already act on an unowned page.

Work around this where possible, but the need to acquire extra page
references is a fair hint that references should have been acquired in
other places instead.

Signed-off-by: Jan Beulich <***@suse.com>
Acked-by: Andrew Cooper <***@citrix.com>
Acked-by: Tamas K Lengyel <***@tklengyel.com>
---
xen/arch/x86/mm/mem_sharing.c | 32 +++++++++++++++++++++++++++++---
xen/common/memory.c | 26 +++++++++++++++++++++-----
2 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index be09c8871a..f4c5074849 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -964,6 +964,15 @@ static int share_pages(struct domain *sd, gfn_t sgfn, shr_handle_t sh,
goto err_out;
}

+ /* Acquire an extra reference, for the freeing below to be safe. */
+ if ( !get_page(cpage, cd) )
+ {
+ ret = -EOVERFLOW;
+ mem_sharing_page_unlock(secondpg);
+ mem_sharing_page_unlock(firstpg);
+ goto err_out;
+ }
+
/* Merge the lists together */
rmap_seed_iterator(cpage, &ri);
while ( (gfn = rmap_iterate(cpage, &ri)) != NULL)
@@ -993,6 +1002,7 @@ static int share_pages(struct domain *sd, gfn_t sgfn, shr_handle_t sh,
/* Free the client page */
if(test_and_clear_bit(_PGC_allocated, &cpage->count_info))
put_page(cpage);
+ put_page(cpage);

/* We managed to free a domain page. */
atomic_dec(&nr_shared_mfns);
@@ -1066,9 +1076,16 @@ int mem_sharing_add_to_physmap(struct domain *sd, unsigned long sgfn, shr_handle
if ( mfn_valid(cmfn) )
{
struct page_info *cpage = mfn_to_page(cmfn);
- ASSERT(cpage != NULL);
+
+ if ( !get_page(cpage, cd) )
+ {
+ domain_crash(cd);
+ ret = -EOVERFLOW;
+ goto err_unlock;
+ }
if ( test_and_clear_bit(_PGC_allocated, &cpage->count_info) )
put_page(cpage);
+ put_page(cpage);
}
}
}
@@ -1153,9 +1170,18 @@ int __mem_sharing_unshare_page(struct domain *d,
mem_sharing_gfn_destroy(page, d, gfn_info);
put_page_and_type(page);
mem_sharing_page_unlock(page);
- if ( last_gfn &&
- test_and_clear_bit(_PGC_allocated, &page->count_info) )
+ if ( last_gfn )
+ {
+ if ( !get_page(page, d) )
+ {
+ put_gfn(d, gfn);
+ domain_crash(d);
+ return -EOVERFLOW;
+ }
+ if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
+ put_page(page);
put_page(page);
+ }
put_gfn(d, gfn);

return 0;
diff --git a/xen/common/memory.c b/xen/common/memory.c
index e894eba672..58194b9dd4 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -313,20 +313,36 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)

if ( unlikely(p2m_is_paging(p2mt)) )
{
+ /*
+ * If the page hasn't yet been paged out, there is an
+ * actual page that needs to be released.
+ */
+ if ( p2mt == p2m_ram_paging_out )
+ {
+ ASSERT(mfn_valid(mfn));
+ page = mfn_to_page(mfn);
+ rc = -ENXIO;
+ if ( !get_page(page, d) )
+ goto out_put_gfn;
+ }
+ else
+ page = NULL;
+
rc = guest_physmap_remove_page(d, _gfn(gmfn), mfn, 0);
if ( rc )
+ {
+ if ( page )
+ put_page(page);
goto out_put_gfn;
+ }

put_gfn(d, gmfn);

- /* If the page hasn't yet been paged out, there is an
- * actual page that needs to be released. */
- if ( p2mt == p2m_ram_paging_out )
+ if ( page )
{
- ASSERT(mfn_valid(mfn));
- page = mfn_to_page(mfn);
if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
put_page(page);
+ put_page(page);
}
p2m_mem_paging_drop_page(d, gmfn, p2mt);

--
generated by git-patchbot for /home/xen/git/xen.git#staging

Loading...