xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* Xen Security Advisory 222 - stale P2M mappings due to insufficient error checking
@ 2017-06-20 12:00 Xen.org security team
  0 siblings, 0 replies; only message in thread
From: Xen.org security team @ 2017-06-20 12:00 UTC (permalink / raw)
  To: xen-announce, xen-devel, xen-users, oss-security; +Cc: Xen.org security team

[-- Attachment #1: Type: text/plain, Size: 4340 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

                    Xen Security Advisory XSA-222
                              version 2

         stale P2M mappings due to insufficient error checking

UPDATES IN VERSION 2
====================

Public release.

ISSUE DESCRIPTION
=================

Certain actions require removing pages from a guest's P2M
(Physical-to-Machine) mapping.  When large pages are in use to map
guest pages in the 2nd-stage page tables, such a removal operation may
incur a memory allocation (to replace a large mapping with individual
smaller ones).  If this allocation fails, these errors are ignored by
the callers, which would then continue and (for example) free the
referenced page for reuse.  This leaves the guest with a mapping to a
page it shouldn't have access to.

The allocation involved comes from a separate pool of memory created
when the domain is created; under normal operating conditions it never
fails, but a malicious guest may be able to engineer situations where
this pool is exhausted.

IMPACT
======

A malicious guest may be able to access memory it doesn't own,
potentially allowing privilege escalation, host crashes, or
information leakage.

VULNERABLE SYSTEMS
==================

Xen versions from at least 3.2 onwards are vulnerable.  Older versions
have not been inspected.

Both x86 and ARM systems are vulnerable.

On x86 systems, only HVM guests can leverage the vulnerability.

MITIGATION
==========

On x86, specifying "hap_1gb=0 hap_2mb=0" on the hypervisor command
line will avoid the vulnerability.

Alternatively, running all x86 HVM guests in shadow mode will also
avoid this vulnerability.  (For example, by specifying "hap=0" in the
xl domain configuration file.)

There is no known mitigation on ARM systems.

CREDITS
=======

This issue was discovered by Julien Grall of ARM.

RESOLUTION
==========

Applying the appropriate pair of attached patches resolves this issue.

xsa222-[12].patch                        xen-unstable
xsa222-1.patch, xsa222-2-4.8.patch       Xen 4.8.x
xsa222-[12]-4.7.patch                    Xen 4.7.x
xsa222-[12]-4.6.patch                    Xen 4.6.x
xsa222-1-4.6.patch, xsa222-2-4.5.patch   Xen 4.5.x

$ sha256sum xsa222*
8bd8807ee1cfe01c86194f5d5be38618ba5e0c1206667bb119ed952e5d155c1a  xsa222-1.patch
9288dfcae1f37e6c8f13910046f43ec161710abb7c94a9346b7e0eaba3258ccd  xsa222-1-4.6.patch
ebc2c070bad8012a196e984b568a72e013ff072bb077870508f09ed053c1a4c2  xsa222-1-4.7.patch
ee320b37b365cb3b6660e559902ff8bb50657b2a28ff0fa7ebaf9ffd33fc0942  xsa222-2.patch
97768f4fe564f702de8e4aebd0c4d24858814ebbb7be532b376cfae7ad6834a4  xsa222-2-4.5.patch
4142f76673b996b65301d52216cbf56e27b0c86e5607f6a9eb18dcc7df3f6343  xsa222-2-4.6.patch
a640e190b32e82f5ec7ee4968bf8b9f22137e8379314cc9a29556637c3dc8e87  xsa222-2-4.7.patch
ab43bd590139bed53957b3b37b854183c69bee26cf7cb00900e3f4a150d067a5  xsa222-2-4.8.patch
$

DEPLOYMENT DURING EMBARGO
=========================

Deployment of the patches and/or mitigations described above (or
others which are substantially similar) is permitted during the
embargo, even on public-facing systems with untrusted guest users and
administrators.

But: Distribution of updated software is prohibited (except to other
members of the predisclosure list).

Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.

(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable.  This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)

For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
  http://www.xenproject.org/security-policy.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBCAAGBQJZSQ3UAAoJEIP+FMlX6CvZUd8H/0Lre7nvQJ+AWb5f9ztcOHcM
Yi+ztFhfKhi9eLrJTGSQqso5rf4Fqf96E+J0UKV5eiI4/u/tRYdhb2kVsv0cwmWR
8npcnpsacTqIzPttTtwiJ4pCh7JM5/OMmEJtuBZHqgw21nCOzIEQjPJTsW0nnnfq
uh20P15sf8ag1mT0N17WuNV1mxdbS6ZqpZMwTYSJfp409oXftsfBkHeH1a9ajdf/
yFZbJQpJ9eizRfZSxmSGMa02V90zQp9vnHhMm1hpy+RrywRysfAVwv4cfIeduo1t
6R3qS+2gAR5YgDvISurBJLAcK1Q0p1qxH5JQd3sYCOPeX3qbbvZ2wDmqPclxa4s=
=iwX3
-----END PGP SIGNATURE-----

[-- Attachment #2: xsa222-1.patch --]
[-- Type: application/octet-stream, Size: 4227 bytes --]

From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: xen/memory: Fix return value handing of guest_remove_page()

Despite the description in mm.h, guest_remove_page() previously returned 0 for
paging errors.

Switch guest_remove_page() to having regular 0/-error semantics, and propagate
the return values from clear_mmio_p2m_entry() and mem_sharing_unshare_page()
to the callers (although decrease_reservation() is the only caller which
currently cares).

This is part of XSA-222.

Reported-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff --git a/xen/common/memory.c b/xen/common/memory.c
index 52879e7..a40bc1c 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -265,6 +265,7 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
     p2m_type_t p2mt;
 #endif
     mfn_t mfn;
+    int rc;
 
 #ifdef CONFIG_X86
     mfn = get_gfn_query(d, gmfn, &p2mt);
@@ -282,13 +283,15 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
                 put_page(page);
         }
         p2m_mem_paging_drop_page(d, gmfn, p2mt);
-        return 1;
+
+        return 0;
     }
     if ( p2mt == p2m_mmio_direct )
     {
-        clear_mmio_p2m_entry(d, gmfn, mfn, 0);
+        rc = clear_mmio_p2m_entry(d, gmfn, mfn, PAGE_ORDER_4K);
         put_gfn(d, gmfn);
-        return 1;
+
+        return rc;
     }
 #else
     mfn = gfn_to_mfn(d, _gfn(gmfn));
@@ -298,21 +301,25 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
         put_gfn(d, gmfn);
         gdprintk(XENLOG_INFO, "Domain %u page number %lx invalid\n",
                 d->domain_id, gmfn);
-        return 0;
+
+        return -EINVAL;
     }
             
 #ifdef CONFIG_X86
     if ( p2m_is_shared(p2mt) )
     {
-        /* Unshare the page, bail out on error. We unshare because 
-         * we might be the only one using this shared page, and we
-         * need to trigger proper cleanup. Once done, this is 
-         * like any other page. */
-        if ( mem_sharing_unshare_page(d, gmfn, 0) )
+        /*
+         * Unshare the page, bail out on error. We unshare because we
+         * might be the only one using this shared page, and we need to
+         * trigger proper cleanup. Once done, this is like any other page.
+         */
+        rc = mem_sharing_unshare_page(d, gmfn, 0);
+        if ( rc )
         {
             put_gfn(d, gmfn);
             (void)mem_sharing_notify_enomem(d, gmfn, 0);
-            return 0;
+
+            return rc;
         }
         /* Maybe the mfn changed */
         mfn = get_gfn_query_unlocked(d, gmfn, &p2mt);
@@ -325,7 +332,8 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
     {
         put_gfn(d, gmfn);
         gdprintk(XENLOG_INFO, "Bad page free for domain %u\n", d->domain_id);
-        return 0;
+
+        return -ENXIO;
     }
 
     if ( test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
@@ -348,7 +356,7 @@ int guest_remove_page(struct domain *d, unsigned long gmfn)
     put_page(page);
     put_gfn(d, gmfn);
 
-    return 1;
+    return 0;
 }
 
 static void decrease_reservation(struct memop_args *a)
@@ -392,7 +400,7 @@ static void decrease_reservation(struct memop_args *a)
             continue;
 
         for ( j = 0; j < (1 << a->extent_order); j++ )
-            if ( !guest_remove_page(a->domain, gmfn + j) )
+            if ( guest_remove_page(a->domain, gmfn + j) )
                 goto out;
     }
 
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 88de3c1..b367930 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -553,9 +553,8 @@ int xenmem_add_to_physmap_one(struct domain *d, unsigned int space,
                               union xen_add_to_physmap_batch_extra extra,
                               unsigned long idx, gfn_t gfn);
 
-/* Returns 1 on success, 0 on error, negative if the ring
- * for event propagation is full in the presence of paging */
-int guest_remove_page(struct domain *d, unsigned long gfn);
+/* Returns 0 on success, or negative on error. */
+int guest_remove_page(struct domain *d, unsigned long gmfn);
 
 #define RAM_TYPE_CONVENTIONAL 0x00000001
 #define RAM_TYPE_RESERVED     0x00000002

[-- Attachment #3: xsa222-1-4.6.patch --]
[-- Type: application/octet-stream, Size: 3851 bytes --]

From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: xen/memory: Fix return value handing of guest_remove_page()

Despite the description in mm.h, guest_remove_page() previously returned 0 for
paging errors.

Switch guest_remove_page() to having regular 0/-error semantics, and propagate
the return values from clear_mmio_p2m_entry() and mem_sharing_unshare_page()
to the callers (although decrease_reservation() is the only caller which
currently cares).

This is part of XSA-222.

Reported-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -240,6 +240,7 @@ int guest_remove_page(struct domain *d,
     p2m_type_t p2mt;
 #endif
     unsigned long mfn;
+    int rc;
 
 #ifdef CONFIG_X86
     mfn = mfn_x(get_gfn_query(d, gmfn, &p2mt)); 
@@ -257,13 +258,15 @@ int guest_remove_page(struct domain *d,
                 put_page(page);
         }
         p2m_mem_paging_drop_page(d, gmfn, p2mt);
-        return 1;
+
+        return 0;
     }
     if ( p2mt == p2m_mmio_direct )
     {
-        clear_mmio_p2m_entry(d, gmfn, _mfn(mfn));
+        rc = clear_mmio_p2m_entry(d, gmfn, _mfn(mfn));
         put_gfn(d, gmfn);
-        return 1;
+
+        return rc;
     }
 #else
     mfn = gmfn_to_mfn(d, gmfn);
@@ -273,21 +276,25 @@ int guest_remove_page(struct domain *d,
         put_gfn(d, gmfn);
         gdprintk(XENLOG_INFO, "Domain %u page number %lx invalid\n",
                 d->domain_id, gmfn);
-        return 0;
+
+        return -EINVAL;
     }
             
 #ifdef CONFIG_X86
     if ( p2m_is_shared(p2mt) )
     {
-        /* Unshare the page, bail out on error. We unshare because 
-         * we might be the only one using this shared page, and we
-         * need to trigger proper cleanup. Once done, this is 
-         * like any other page. */
-        if ( mem_sharing_unshare_page(d, gmfn, 0) )
+        /*
+         * Unshare the page, bail out on error. We unshare because we
+         * might be the only one using this shared page, and we need to
+         * trigger proper cleanup. Once done, this is like any other page.
+         */
+        rc = mem_sharing_unshare_page(d, gmfn, 0);
+        if ( rc )
         {
             put_gfn(d, gmfn);
             (void)mem_sharing_notify_enomem(d, gmfn, 0);
-            return 0;
+
+            return rc;
         }
         /* Maybe the mfn changed */
         mfn = mfn_x(get_gfn_query_unlocked(d, gmfn, &p2mt));
@@ -300,7 +307,8 @@ int guest_remove_page(struct domain *d,
     {
         put_gfn(d, gmfn);
         gdprintk(XENLOG_INFO, "Bad page free for domain %u\n", d->domain_id);
-        return 0;
+
+        return -ENXIO;
     }
 
     if ( test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
@@ -314,7 +322,7 @@ int guest_remove_page(struct domain *d,
     put_page(page);
     put_gfn(d, gmfn);
 
-    return 1;
+    return 0;
 }
 
 static void decrease_reservation(struct memop_args *a)
@@ -365,7 +373,7 @@ static void decrease_reservation(struct
             continue;
 
         for ( j = 0; j < (1 << a->extent_order); j++ )
-            if ( !guest_remove_page(a->domain, gmfn + j) )
+            if ( guest_remove_page(a->domain, gmfn + j) )
                 goto out;
     }
 
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -447,8 +447,7 @@ int xenmem_add_to_physmap_one(struct dom
                               domid_t foreign_domid,
                               unsigned long idx, xen_pfn_t gpfn);
 
-/* Returns 1 on success, 0 on error, negative if the ring
- * for event propagation is full in the presence of paging */
+/* Returns 0 on success, or negative on error. */
 int guest_remove_page(struct domain *d, unsigned long gmfn);
 
 #define RAM_TYPE_CONVENTIONAL 0x00000001

[-- Attachment #4: xsa222-1-4.7.patch --]
[-- Type: application/octet-stream, Size: 3890 bytes --]

From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: xen/memory: Fix return value handing of guest_remove_page()

Despite the description in mm.h, guest_remove_page() previously returned 0 for
paging errors.

Switch guest_remove_page() to having regular 0/-error semantics, and propagate
the return values from clear_mmio_p2m_entry() and mem_sharing_unshare_page()
to the callers (although decrease_reservation() is the only caller which
currently cares).

This is part of XSA-222.

Reported-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -244,6 +244,7 @@ int guest_remove_page(struct domain *d,
     p2m_type_t p2mt;
 #endif
     unsigned long mfn;
+    int rc;
 
 #ifdef CONFIG_X86
     mfn = mfn_x(get_gfn_query(d, gmfn, &p2mt)); 
@@ -261,13 +262,15 @@ int guest_remove_page(struct domain *d,
                 put_page(page);
         }
         p2m_mem_paging_drop_page(d, gmfn, p2mt);
-        return 1;
+
+        return 0;
     }
     if ( p2mt == p2m_mmio_direct )
     {
-        clear_mmio_p2m_entry(d, gmfn, _mfn(mfn), 0);
+        rc = clear_mmio_p2m_entry(d, gmfn, _mfn(mfn), PAGE_ORDER_4K);
         put_gfn(d, gmfn);
-        return 1;
+
+        return rc;
     }
 #else
     mfn = gmfn_to_mfn(d, gmfn);
@@ -277,21 +280,25 @@ int guest_remove_page(struct domain *d,
         put_gfn(d, gmfn);
         gdprintk(XENLOG_INFO, "Domain %u page number %lx invalid\n",
                 d->domain_id, gmfn);
-        return 0;
+
+        return -EINVAL;
     }
             
 #ifdef CONFIG_X86
     if ( p2m_is_shared(p2mt) )
     {
-        /* Unshare the page, bail out on error. We unshare because 
-         * we might be the only one using this shared page, and we
-         * need to trigger proper cleanup. Once done, this is 
-         * like any other page. */
-        if ( mem_sharing_unshare_page(d, gmfn, 0) )
+        /*
+         * Unshare the page, bail out on error. We unshare because we
+         * might be the only one using this shared page, and we need to
+         * trigger proper cleanup. Once done, this is like any other page.
+         */
+        rc = mem_sharing_unshare_page(d, gmfn, 0);
+        if ( rc )
         {
             put_gfn(d, gmfn);
             (void)mem_sharing_notify_enomem(d, gmfn, 0);
-            return 0;
+
+            return rc;
         }
         /* Maybe the mfn changed */
         mfn = mfn_x(get_gfn_query_unlocked(d, gmfn, &p2mt));
@@ -304,7 +311,8 @@ int guest_remove_page(struct domain *d,
     {
         put_gfn(d, gmfn);
         gdprintk(XENLOG_INFO, "Bad page free for domain %u\n", d->domain_id);
-        return 0;
+
+        return -ENXIO;
     }
 
     if ( test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
@@ -327,7 +335,7 @@ int guest_remove_page(struct domain *d,
     put_page(page);
     put_gfn(d, gmfn);
 
-    return 1;
+    return 0;
 }
 
 static void decrease_reservation(struct memop_args *a)
@@ -371,7 +379,7 @@ static void decrease_reservation(struct
             continue;
 
         for ( j = 0; j < (1 << a->extent_order); j++ )
-            if ( !guest_remove_page(a->domain, gmfn + j) )
+            if ( guest_remove_page(a->domain, gmfn + j) )
                 goto out;
     }
 
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -509,8 +509,7 @@ int xenmem_add_to_physmap_one(struct dom
                               union xen_add_to_physmap_batch_extra extra,
                               unsigned long idx, xen_pfn_t gpfn);
 
-/* Returns 1 on success, 0 on error, negative if the ring
- * for event propagation is full in the presence of paging */
+/* Returns 0 on success, or negative on error. */
 int guest_remove_page(struct domain *d, unsigned long gmfn);
 
 #define RAM_TYPE_CONVENTIONAL 0x00000001

[-- Attachment #5: xsa222-2.patch --]
[-- Type: application/octet-stream, Size: 14098 bytes --]

From: Jan Beulich <jbeulich@suse.com>
Subject: guest_physmap_remove_page() needs its return value checked

Callers, namely such subsequently freeing the page, must not blindly
assume success - the function may namely fail when needing to shatter a
super page, but there not being memory available for the then needed
intermediate page table.

As it happens, guest_remove_page() callers now also all check the
return value.

Furthermore a missed put_gfn() on an error path in gnttab_transfer() is
also being taken care of.

This is part of XSA-222.

Reported-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
v4: Also make guest_remove_page() __must_check.
v3: Rebase over new precursor patch.
v2: Also avoid bypassing put_gfn() on an error path in
    gnttab_transfer(). As a result also fold some error paths there
    (hopefully making it easier to verify that no step is omitted).
    ARM changes from Julien. Move declaration to p2m-common.h.

--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1392,13 +1392,14 @@ int replace_grant_host_mapping(unsigned
 {
     gfn_t gfn = _gfn(addr >> PAGE_SHIFT);
     struct domain *d = current->domain;
+    int rc;
 
     if ( new_addr != 0 || (flags & GNTMAP_contains_pte) )
         return GNTST_general_error;
 
-    guest_physmap_remove_page(d, gfn, _mfn(mfn), 0);
+    rc = guest_physmap_remove_page(d, gfn, _mfn(mfn), 0);
 
-    return GNTST_okay;
+    return rc ? GNTST_general_error : GNTST_okay;
 }
 
 bool is_iomem_page(mfn_t mfn)
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1137,11 +1137,10 @@ int guest_physmap_add_entry(struct domai
     return p2m_insert_mapping(d, gfn, (1 << page_order), mfn, t);
 }
 
-void guest_physmap_remove_page(struct domain *d,
-                               gfn_t gfn,
-                               mfn_t mfn, unsigned int page_order)
+int guest_physmap_remove_page(struct domain *d, gfn_t gfn, mfn_t mfn,
+                              unsigned int page_order)
 {
-    p2m_remove_mapping(d, gfn, (1 << page_order), mfn);
+    return p2m_remove_mapping(d, gfn, (1 << page_order), mfn);
 }
 
 static int p2m_alloc_table(struct domain *d)
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -815,7 +815,15 @@ int arch_domain_soft_reset(struct domain
         ret = -ENOMEM;
         goto exit_put_gfn;
     }
-    guest_physmap_remove_page(d, _gfn(gfn), _mfn(mfn), PAGE_ORDER_4K);
+
+    ret = guest_physmap_remove_page(d, _gfn(gfn), _mfn(mfn), PAGE_ORDER_4K);
+    if ( ret )
+    {
+        printk(XENLOG_G_ERR "Failed to remove Dom%d's shared_info frame %lx\n",
+               d->domain_id, gfn);
+        free_domheap_page(new_page);
+        goto exit_put_gfn;
+    }
 
     ret = guest_physmap_add_page(d, _gfn(gfn), _mfn(page_to_mfn(new_page)),
                                  PAGE_ORDER_4K);
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -267,8 +267,9 @@ bool_t is_ioreq_server_page(struct domai
 static void hvm_remove_ioreq_gmfn(
     struct domain *d, struct hvm_ioreq_page *iorp)
 {
-    guest_physmap_remove_page(d, _gfn(iorp->gmfn),
-                              _mfn(page_to_mfn(iorp->page)), 0);
+    if ( guest_physmap_remove_page(d, _gfn(iorp->gmfn),
+                                   _mfn(page_to_mfn(iorp->page)), 0) )
+        domain_crash(d);
     clear_page(iorp->va);
 }
 
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4320,7 +4320,11 @@ static int replace_grant_p2m_mapping(
                  type, mfn_x(old_mfn), frame);
         return GNTST_general_error;
     }
-    guest_physmap_remove_page(d, _gfn(gfn), _mfn(frame), PAGE_ORDER_4K);
+    if ( guest_physmap_remove_page(d, _gfn(gfn), _mfn(frame), PAGE_ORDER_4K) )
+    {
+        put_gfn(d, gfn);
+        return GNTST_general_error;
+    }
 
     put_gfn(d, gfn);
     return GNTST_okay;
@@ -4850,7 +4854,7 @@ int xenmem_add_to_physmap_one(
     struct page_info *page = NULL;
     unsigned long gfn = 0; /* gcc ... */
     unsigned long prev_mfn, mfn = 0, old_gpfn;
-    int rc;
+    int rc = 0;
     p2m_type_t p2mt;
 
     switch ( space )
@@ -4924,25 +4928,30 @@ int xenmem_add_to_physmap_one(
     {
         if ( is_xen_heap_mfn(prev_mfn) )
             /* Xen heap frames are simply unhooked from this phys slot. */
-            guest_physmap_remove_page(d, gpfn, _mfn(prev_mfn), PAGE_ORDER_4K);
+            rc = guest_physmap_remove_page(d, gpfn, _mfn(prev_mfn), PAGE_ORDER_4K);
         else
             /* Normal domain memory is freed, to avoid leaking memory. */
-            guest_remove_page(d, gfn_x(gpfn));
+            rc = guest_remove_page(d, gfn_x(gpfn));
     }
     /* In the XENMAPSPACE_gmfn case we still hold a ref on the old page. */
     put_gfn(d, gfn_x(gpfn));
 
+    if ( rc )
+        goto put_both;
+
     /* Unmap from old location, if any. */
     old_gpfn = get_gpfn_from_mfn(mfn);
     ASSERT( old_gpfn != SHARED_M2P_ENTRY );
     if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range )
         ASSERT( old_gpfn == gfn );
     if ( old_gpfn != INVALID_M2P_ENTRY )
-        guest_physmap_remove_page(d, _gfn(old_gpfn), _mfn(mfn), PAGE_ORDER_4K);
+        rc = guest_physmap_remove_page(d, _gfn(old_gpfn), _mfn(mfn), PAGE_ORDER_4K);
 
     /* Map at new location. */
-    rc = guest_physmap_add_page(d, gpfn, _mfn(mfn), PAGE_ORDER_4K);
+    if ( !rc )
+        rc = guest_physmap_add_page(d, gpfn, _mfn(mfn), PAGE_ORDER_4K);
 
+ put_both:
     /* In the XENMAPSPACE_gmfn, we took a ref of the gfn at the top */
     if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range )
         put_gfn(d, gfn);
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2585,10 +2585,12 @@ int p2m_add_foreign(struct domain *tdom,
     {
         if ( is_xen_heap_mfn(mfn_x(prev_mfn)) )
             /* Xen heap frames are simply unhooked from this phys slot */
-            guest_physmap_remove_page(tdom, _gfn(gpfn), prev_mfn, 0);
+            rc = guest_physmap_remove_page(tdom, _gfn(gpfn), prev_mfn, 0);
         else
             /* Normal domain memory is freed, to avoid leaking memory. */
-            guest_remove_page(tdom, gpfn);
+            rc = guest_remove_page(tdom, gpfn);
+        if ( rc )
+            goto put_both;
     }
     /*
      * Create the new mapping. Can't use guest_physmap_add_page() because it
@@ -2601,6 +2603,7 @@ int p2m_add_foreign(struct domain *tdom,
                  "gpfn:%lx mfn:%lx fgfn:%lx td:%d fd:%d\n",
                  gpfn, mfn_x(mfn), fgfn, tdom->domain_id, fdom->domain_id);
 
+ put_both:
     put_page(page);
 
     /*
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1768,6 +1768,7 @@ gnttab_transfer(
     for ( i = 0; i < count; i++ )
     {
         bool_t okay;
+        int rc;
 
         if (i && hypercall_preempt_check())
             return i;
@@ -1818,27 +1819,33 @@ gnttab_transfer(
             goto copyback;
         }
 
-        guest_physmap_remove_page(d, _gfn(gop.mfn), _mfn(mfn), 0);
+        rc = guest_physmap_remove_page(d, _gfn(gop.mfn), _mfn(mfn), 0);
         gnttab_flush_tlb(d);
+        if ( rc )
+        {
+            gdprintk(XENLOG_INFO,
+                     "gnttab_transfer: can't remove GFN %"PRI_xen_pfn" (MFN %lx)\n",
+                     gop.mfn, mfn);
+            gop.status = GNTST_general_error;
+            goto put_gfn_and_copyback;
+        }
 
         /* Find the target domain. */
         if ( unlikely((e = rcu_lock_domain_by_id(gop.domid)) == NULL) )
         {
-            put_gfn(d, gop.mfn);
             gdprintk(XENLOG_INFO, "gnttab_transfer: can't find domain %d\n",
                     gop.domid);
-            page->count_info &= ~(PGC_count_mask|PGC_allocated);
-            free_domheap_page(page);
             gop.status = GNTST_bad_domain;
-            goto copyback;
+            goto put_gfn_and_copyback;
         }
 
         if ( xsm_grant_transfer(XSM_HOOK, d, e) )
         {
-            put_gfn(d, gop.mfn);
             gop.status = GNTST_permission_denied;
         unlock_and_copyback:
             rcu_unlock_domain(e);
+        put_gfn_and_copyback:
+            put_gfn(d, gop.mfn);
             page->count_info &= ~(PGC_count_mask|PGC_allocated);
             free_domheap_page(page);
             goto copyback;
@@ -1887,12 +1894,8 @@ gnttab_transfer(
                          "Transferee (d%d) has no headroom (tot %u, max %u)\n",
                          e->domain_id, e->tot_pages, e->max_pages);
 
-            rcu_unlock_domain(e);
-            put_gfn(d, gop.mfn);
-            page->count_info &= ~(PGC_count_mask|PGC_allocated);
-            free_domheap_page(page);
             gop.status = GNTST_general_error;
-            goto copyback;
+            goto unlock_and_copyback;
         }
 
         /* Okay, add the page to 'e'. */
@@ -1921,13 +1924,8 @@ gnttab_transfer(
 
             if ( drop_dom_ref )
                 put_domain(e);
-            rcu_unlock_domain(e);
-
-            put_gfn(d, gop.mfn);
-            page->count_info &= ~(PGC_count_mask|PGC_allocated);
-            free_domheap_page(page);
             gop.status = GNTST_general_error;
-            goto copyback;
+            goto unlock_and_copyback;
         }
 
         page_list_add_tail(page, &e->page_list);
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -271,8 +271,12 @@ int guest_remove_page(struct domain *d,
     mfn = get_gfn_query(d, gmfn, &p2mt);
     if ( unlikely(p2m_is_paging(p2mt)) )
     {
-        guest_physmap_remove_page(d, _gfn(gmfn), mfn, 0);
+        rc = guest_physmap_remove_page(d, _gfn(gmfn), mfn, 0);
         put_gfn(d, gmfn);
+
+        if ( rc )
+            return rc;
+
         /* 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 )
@@ -336,7 +340,9 @@ int guest_remove_page(struct domain *d,
         return -ENXIO;
     }
 
-    if ( test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
+    rc = guest_physmap_remove_page(d, _gfn(gmfn), mfn, 0);
+
+    if ( !rc && test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
         put_page_and_type(page);
 
     /*
@@ -347,16 +353,14 @@ int guest_remove_page(struct domain *d,
      * For this purpose (and to match populate_physmap() behavior), the page
      * is kept allocated.
      */
-    if ( !is_domain_direct_mapped(d) &&
+    if ( !rc && !is_domain_direct_mapped(d) &&
          test_and_clear_bit(_PGC_allocated, &page->count_info) )
         put_page(page);
 
-    guest_physmap_remove_page(d, _gfn(gmfn), mfn, 0);
-
     put_page(page);
     put_gfn(d, gmfn);
 
-    return 0;
+    return rc;
 }
 
 static void decrease_reservation(struct memop_args *a)
@@ -591,7 +595,8 @@ static long memory_exchange(XEN_GUEST_HA
             gfn = mfn_to_gmfn(d, mfn);
             /* Pages were unshared above */
             BUG_ON(SHARED_M2P(gfn));
-            guest_physmap_remove_page(d, _gfn(gfn), _mfn(mfn), 0);
+            if ( guest_physmap_remove_page(d, _gfn(gfn), _mfn(mfn), 0) )
+                domain_crash(d);
             put_page(page);
         }
 
@@ -1150,8 +1155,8 @@ long do_memory_op(unsigned long cmd, XEN
         page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC);
         if ( page )
         {
-            guest_physmap_remove_page(d, _gfn(xrfp.gpfn),
-                                      _mfn(page_to_mfn(page)), 0);
+            rc = guest_physmap_remove_page(d, _gfn(xrfp.gpfn),
+                                           _mfn(page_to_mfn(page)), 0);
             put_page(page);
         }
         else
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2775,9 +2775,7 @@ static int __must_check arm_smmu_unmap_p
 	if ( !is_domain_direct_mapped(d) )
 		return -EINVAL;
 
-	guest_physmap_remove_page(d, _gfn(gfn), _mfn(gfn), 0);
-
-	return 0;
+	return guest_physmap_remove_page(d, _gfn(gfn), _mfn(gfn), 0);
 }
 
 static const struct iommu_ops arm_smmu_iommu_ops = {
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -258,10 +258,6 @@ static inline int guest_physmap_add_page
     return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw);
 }
 
-void guest_physmap_remove_page(struct domain *d,
-                               gfn_t gfn,
-                               mfn_t mfn, unsigned int page_order);
-
 mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn);
 
 /*
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -579,10 +579,6 @@ static inline int guest_physmap_add_page
     return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw);
 }
 
-/* Remove a page from a domain's p2m table */
-int guest_physmap_remove_page(struct domain *d,
-                              gfn_t gfn, mfn_t mfn, unsigned int page_order);
-
 /* Set a p2m range as populate-on-demand */
 int guest_physmap_mark_populate_on_demand(struct domain *d, unsigned long gfn,
                                           unsigned int order);
--- a/xen/include/xen/p2m-common.h
+++ b/xen/include/xen/p2m-common.h
@@ -1,6 +1,13 @@
 #ifndef _XEN_P2M_COMMON_H
 #define _XEN_P2M_COMMON_H
 
+#include <xen/mm.h>
+
+/* Remove a page from a domain's p2m table */
+int __must_check
+guest_physmap_remove_page(struct domain *d, gfn_t gfn, mfn_t mfn,
+                          unsigned int page_order);
+
 /* Map MMIO regions in the p2m: start_gfn and nr describe the range in
  *  * the guest physical address space to map, starting from the machine
  *   * frame number mfn. */
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -554,7 +554,7 @@ int xenmem_add_to_physmap_one(struct dom
                               unsigned long idx, gfn_t gfn);
 
 /* Returns 0 on success, or negative on error. */
-int guest_remove_page(struct domain *d, unsigned long gmfn);
+int __must_check guest_remove_page(struct domain *d, unsigned long gmfn);
 
 #define RAM_TYPE_CONVENTIONAL 0x00000001
 #define RAM_TYPE_RESERVED     0x00000002

[-- Attachment #6: xsa222-2-4.5.patch --]
[-- Type: application/octet-stream, Size: 13715 bytes --]

From: Jan Beulich <jbeulich@suse.com>
Subject: guest_physmap_remove_page() needs its return value checked

Callers, namely such subsequently freeing the page, must not blindly
assume success - the function may namely fail when needing to shatter a
super page, but there not being memory available for the then needed
intermediate page table.

As it happens, guest_remove_page() callers now also all check the
return value.

Furthermore a missed put_gfn() on an error path in gnttab_transfer() is
also being taken care of.

This is part of XSA-222.

Reported-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1268,13 +1268,14 @@ int replace_grant_host_mapping(unsigned
 {
     unsigned long gfn = (unsigned long)(addr >> PAGE_SHIFT);
     struct domain *d = current->domain;
+    int rc;
 
     if ( new_addr != 0 || (flags & GNTMAP_contains_pte) )
         return GNTST_general_error;
 
-    guest_physmap_remove_page(d, gfn, mfn, 0);
+    rc = guest_physmap_remove_page(d, gfn, mfn, 0);
 
-    return GNTST_okay;
+    return rc ? GNTST_general_error : GNTST_okay;
 }
 
 int is_iomem_page(unsigned long mfn)
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -976,14 +976,13 @@ int guest_physmap_add_entry(struct domai
                              pfn_to_paddr(mfn), MATTR_MEM, t);
 }
 
-void guest_physmap_remove_page(struct domain *d,
-                               unsigned long gpfn,
-                               unsigned long mfn, unsigned int page_order)
+int guest_physmap_remove_page(struct domain *d, unsigned long gfn,
+                              unsigned long mfn, unsigned int page_order)
 {
-    apply_p2m_changes(d, REMOVE,
-                      pfn_to_paddr(gpfn),
-                      pfn_to_paddr(gpfn + (1<<page_order)),
-                      pfn_to_paddr(mfn), MATTR_MEM, p2m_invalid);
+    return apply_p2m_changes(d, REMOVE,
+                             pfn_to_paddr(gfn),
+                             pfn_to_paddr(gfn + (1 << page_order)),
+                             pfn_to_paddr(mfn), MATTR_MEM, p2m_invalid);
 }
 
 int p2m_alloc_table(struct domain *d)
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -548,8 +548,9 @@ bool_t is_ioreq_server_page(struct domai
 static void hvm_remove_ioreq_gmfn(
     struct domain *d, struct hvm_ioreq_page *iorp)
 {
-    guest_physmap_remove_page(d, iorp->gmfn, 
-                              page_to_mfn(iorp->page), 0);
+    if ( guest_physmap_remove_page(d, iorp->gmfn,
+                                   page_to_mfn(iorp->page), 0) )
+        domain_crash(d);
     clear_page(iorp->va);
 }
 
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4092,7 +4092,11 @@ static int replace_grant_p2m_mapping(
                  type, mfn_x(old_mfn), frame);
         return GNTST_general_error;
     }
-    guest_physmap_remove_page(d, gfn, frame, PAGE_ORDER_4K);
+    if ( guest_physmap_remove_page(d, gfn, frame, PAGE_ORDER_4K) )
+    {
+        put_gfn(d, gfn);
+        return GNTST_general_error;
+    }
 
     put_gfn(d, gfn);
     return GNTST_okay;
@@ -4610,7 +4614,7 @@ int xenmem_add_to_physmap_one(
     struct page_info *page = NULL;
     unsigned long gfn = 0; /* gcc ... */
     unsigned long prev_mfn, mfn = 0, old_gpfn;
-    int rc;
+    int rc = 0;
     p2m_type_t p2mt;
 
     switch ( space )
@@ -4684,25 +4688,30 @@ int xenmem_add_to_physmap_one(
     {
         if ( is_xen_heap_mfn(prev_mfn) )
             /* Xen heap frames are simply unhooked from this phys slot. */
-            guest_physmap_remove_page(d, gpfn, prev_mfn, PAGE_ORDER_4K);
+            rc = guest_physmap_remove_page(d, gpfn, prev_mfn, PAGE_ORDER_4K);
         else
             /* Normal domain memory is freed, to avoid leaking memory. */
-            guest_remove_page(d, gpfn);
+            rc = guest_remove_page(d, gpfn);
     }
     /* In the XENMAPSPACE_gmfn case we still hold a ref on the old page. */
     put_gfn(d, gpfn);
 
+    if ( rc )
+        goto put_both;
+
     /* Unmap from old location, if any. */
     old_gpfn = get_gpfn_from_mfn(mfn);
     ASSERT( old_gpfn != SHARED_M2P_ENTRY );
     if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range )
         ASSERT( old_gpfn == gfn );
     if ( old_gpfn != INVALID_M2P_ENTRY )
-        guest_physmap_remove_page(d, old_gpfn, mfn, PAGE_ORDER_4K);
+        rc = guest_physmap_remove_page(d, old_gpfn, mfn, PAGE_ORDER_4K);
 
     /* Map at new location. */
-    rc = guest_physmap_add_page(d, gpfn, mfn, PAGE_ORDER_4K);
+    if ( !rc )
+        rc = guest_physmap_add_page(d, gpfn, mfn, PAGE_ORDER_4K);
 
+ put_both:
     /* In the XENMAPSPACE_gmfn, we took a ref of the gfn at the top */
     if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range )
         put_gfn(d, gfn);
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -550,14 +550,18 @@ p2m_remove_page(struct p2m_domain *p2m,
                          p2m->default_access);
 }
 
-void
+int
 guest_physmap_remove_page(struct domain *d, unsigned long gfn,
                           unsigned long mfn, unsigned int page_order)
 {
+    int rc;
+
     struct p2m_domain *p2m = p2m_get_hostp2m(d);
     gfn_lock(p2m, gfn, page_order);
-    p2m_remove_page(p2m, gfn, mfn, page_order);
+    rc = p2m_remove_page(p2m, gfn, mfn, page_order);
     gfn_unlock(p2m, gfn, page_order);
+
+    return rc;
 }
 
 int
@@ -2094,10 +2098,12 @@ int p2m_add_foreign(struct domain *tdom,
     {
         if ( is_xen_heap_mfn(prev_mfn) )
             /* Xen heap frames are simply unhooked from this phys slot */
-            guest_physmap_remove_page(tdom, gpfn, prev_mfn, 0);
+            rc = guest_physmap_remove_page(tdom, gpfn, prev_mfn, 0);
         else
             /* Normal domain memory is freed, to avoid leaking memory. */
-            guest_remove_page(tdom, gpfn);
+            rc = guest_remove_page(tdom, gpfn);
+        if ( rc )
+            goto put_both;
     }
     /*
      * Create the new mapping. Can't use guest_physmap_add_page() because it
@@ -2110,6 +2116,7 @@ int p2m_add_foreign(struct domain *tdom,
                  "gpfn:%lx mfn:%lx fgfn:%lx td:%d fd:%d\n",
                  gpfn, mfn, fgfn, tdom->domain_id, fdom->domain_id);
 
+ put_both:
     put_page(page);
 
     /*
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1574,6 +1574,7 @@ gnttab_transfer(
     for ( i = 0; i < count; i++ )
     {
         bool_t okay;
+        int rc;
 
         if (i && hypercall_preempt_check())
             return i;
@@ -1624,27 +1625,33 @@ gnttab_transfer(
             goto copyback;
         }
 
-        guest_physmap_remove_page(d, gop.mfn, mfn, 0);
+        rc = guest_physmap_remove_page(d, gop.mfn, mfn, 0);
         gnttab_flush_tlb(d);
+        if ( rc )
+        {
+            gdprintk(XENLOG_INFO,
+                     "gnttab_transfer: can't remove GFN %"PRI_xen_pfn" (MFN %lx)\n",
+                     gop.mfn, mfn);
+            gop.status = GNTST_general_error;
+            goto put_gfn_and_copyback;
+        }
 
         /* Find the target domain. */
         if ( unlikely((e = rcu_lock_domain_by_id(gop.domid)) == NULL) )
         {
-            put_gfn(d, gop.mfn);
             gdprintk(XENLOG_INFO, "gnttab_transfer: can't find domain %d\n",
                     gop.domid);
-            page->count_info &= ~(PGC_count_mask|PGC_allocated);
-            free_domheap_page(page);
             gop.status = GNTST_bad_domain;
-            goto copyback;
+            goto put_gfn_and_copyback;
         }
 
         if ( xsm_grant_transfer(XSM_HOOK, d, e) )
         {
-            put_gfn(d, gop.mfn);
             gop.status = GNTST_permission_denied;
         unlock_and_copyback:
             rcu_unlock_domain(e);
+        put_gfn_and_copyback:
+            put_gfn(d, gop.mfn);
             page->count_info &= ~(PGC_count_mask|PGC_allocated);
             free_domheap_page(page);
             goto copyback;
@@ -1695,12 +1702,8 @@ gnttab_transfer(
                          "Transferee (d%d) has no headroom (tot %u, max %u)\n",
                          e->domain_id, e->tot_pages, e->max_pages);
 
-            rcu_unlock_domain(e);
-            put_gfn(d, gop.mfn);
-            page->count_info &= ~(PGC_count_mask|PGC_allocated);
-            free_domheap_page(page);
             gop.status = GNTST_general_error;
-            goto copyback;
+            goto unlock_and_copyback;
         }
 
         /* Okay, add the page to 'e'. */
@@ -1729,13 +1732,8 @@ gnttab_transfer(
 
             if ( drop_dom_ref )
                 put_domain(e);
-            rcu_unlock_domain(e);
-
-            put_gfn(d, gop.mfn);
-            page->count_info &= ~(PGC_count_mask|PGC_allocated);
-            free_domheap_page(page);
             gop.status = GNTST_general_error;
-            goto copyback;
+            goto unlock_and_copyback;
         }
 
         page_list_add_tail(page, &e->page_list);
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -246,8 +246,12 @@ int guest_remove_page(struct domain *d,
     mfn = mfn_x(get_gfn_query(d, gmfn, &p2mt)); 
     if ( unlikely(p2m_is_paging(p2mt)) )
     {
-        guest_physmap_remove_page(d, gmfn, mfn, 0);
+        rc = guest_physmap_remove_page(d, gmfn, mfn, 0);
         put_gfn(d, gmfn);
+
+        if ( rc )
+            return rc;
+
         /* 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 )
@@ -311,18 +315,18 @@ int guest_remove_page(struct domain *d,
         return -ENXIO;
     }
 
-    if ( test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
+    rc = guest_physmap_remove_page(d, gmfn, mfn, 0);
+
+    if ( !rc && test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
         put_page_and_type(page);
             
-    if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
+    if ( !rc && test_and_clear_bit(_PGC_allocated, &page->count_info) )
         put_page(page);
 
-    guest_physmap_remove_page(d, gmfn, mfn, 0);
-
     put_page(page);
     put_gfn(d, gmfn);
 
-    return 0;
+    return rc;
 }
 
 static void decrease_reservation(struct memop_args *a)
@@ -563,7 +567,8 @@ static long memory_exchange(XEN_GUEST_HA
             gfn = mfn_to_gmfn(d, mfn);
             /* Pages were unshared above */
             BUG_ON(SHARED_M2P(gfn));
-            guest_physmap_remove_page(d, gfn, mfn, 0);
+            if ( guest_physmap_remove_page(d, gfn, mfn, 0) )
+                domain_crash(d);
             put_page(page);
         }
 
@@ -1020,7 +1025,7 @@ long do_memory_op(unsigned long cmd, XEN
         page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC);
         if ( page )
         {
-            guest_physmap_remove_page(d, xrfp.gpfn, page_to_mfn(page), 0);
+            rc = guest_physmap_remove_page(d, xrfp.gpfn, page_to_mfn(page), 0);
             put_page(page);
         }
         else
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -1571,9 +1571,7 @@ static int arm_smmu_unmap_page(struct do
     if ( !is_domain_direct_mapped(d) )
         return -EINVAL;
 
-    guest_physmap_remove_page(d, gfn, gfn, 0);
-
-    return 0;
+    return guest_physmap_remove_page(d, gfn, gfn, 0);
 }
 
 static const struct iommu_ops arm_smmu_iommu_ops = {
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -141,10 +141,6 @@ static inline int guest_physmap_add_page
     return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw);
 }
 
-void guest_physmap_remove_page(struct domain *d,
-                               unsigned long gpfn,
-                               unsigned long mfn, unsigned int page_order);
-
 unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn);
 
 /*
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -494,11 +494,6 @@ static inline int guest_physmap_add_page
     return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw);
 }
 
-/* Remove a page from a domain's p2m table */
-void guest_physmap_remove_page(struct domain *d,
-                               unsigned long gfn,
-                               unsigned long mfn, unsigned int page_order);
-
 /* Set a p2m range as populate-on-demand */
 int guest_physmap_mark_populate_on_demand(struct domain *d, unsigned long gfn,
                                           unsigned int order);
--- a/xen/include/xen/p2m-common.h
+++ b/xen/include/xen/p2m-common.h
@@ -1,6 +1,7 @@
 #ifndef _XEN_P2M_COMMON_H
 #define _XEN_P2M_COMMON_H
 
+#include <xen/mm.h>
 #include <public/mem_event.h>
 
 /*
@@ -32,6 +33,11 @@ typedef enum {
     /* NOTE: Assumed to be only 4 bits right now on x86. */
 } p2m_access_t;
 
+/* Remove a page from a domain's p2m table */
+int __must_check
+guest_physmap_remove_page(struct domain *d, unsigned long gfn,
+                          unsigned long mfn, unsigned int page_order);
+
 /* Map MMIO regions in the p2m: start_gfn and nr describe the range in
  *  * the guest physical address space to map, starting from the machine
  *   * frame number mfn. */
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -382,7 +382,7 @@ int xenmem_add_to_physmap_one(struct dom
                               unsigned long idx, xen_pfn_t gpfn);
 
 /* Returns 0 on success, or negative on error. */
-int guest_remove_page(struct domain *d, unsigned long gmfn);
+int __must_check guest_remove_page(struct domain *d, unsigned long gmfn);
 
 #define RAM_TYPE_CONVENTIONAL 0x00000001
 #define RAM_TYPE_RESERVED     0x00000002

[-- Attachment #7: xsa222-2-4.6.patch --]
[-- Type: application/octet-stream, Size: 13825 bytes --]

From: Jan Beulich <jbeulich@suse.com>
Subject: guest_physmap_remove_page() needs its return value checked

Callers, namely such subsequently freeing the page, must not blindly
assume success - the function may namely fail when needing to shatter a
super page, but there not being memory available for the then needed
intermediate page table.

As it happens, guest_remove_page() callers now also all check the
return value.

Furthermore a missed put_gfn() on an error path in gnttab_transfer() is
also being taken care of.

This is part of XSA-222.

Reported-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1283,13 +1283,14 @@ int replace_grant_host_mapping(unsigned
 {
     unsigned long gfn = (unsigned long)(addr >> PAGE_SHIFT);
     struct domain *d = current->domain;
+    int rc;
 
     if ( new_addr != 0 || (flags & GNTMAP_contains_pte) )
         return GNTST_general_error;
 
-    guest_physmap_remove_page(d, gfn, mfn, 0);
+    rc = guest_physmap_remove_page(d, gfn, mfn, 0);
 
-    return GNTST_okay;
+    return rc ? GNTST_general_error : GNTST_okay;
 }
 
 int is_iomem_page(unsigned long mfn)
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1176,15 +1176,14 @@ int guest_physmap_add_entry(struct domai
                              d->arch.p2m.default_access);
 }
 
-void guest_physmap_remove_page(struct domain *d,
-                               unsigned long gpfn,
-                               unsigned long mfn, unsigned int page_order)
+int guest_physmap_remove_page(struct domain *d, unsigned long gfn,
+                              unsigned long mfn, unsigned int page_order)
 {
-    apply_p2m_changes(d, REMOVE,
-                      pfn_to_paddr(gpfn),
-                      pfn_to_paddr(gpfn + (1<<page_order)),
-                      pfn_to_paddr(mfn), MATTR_MEM, 0, p2m_invalid,
-                      d->arch.p2m.default_access);
+    return apply_p2m_changes(d, REMOVE,
+                             pfn_to_paddr(gfn),
+                             pfn_to_paddr(gfn + (1 << page_order)),
+                             pfn_to_paddr(mfn), MATTR_MEM, 0, p2m_invalid,
+                             d->arch.p2m.default_access);
 }
 
 int p2m_alloc_table(struct domain *d)
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -427,7 +427,9 @@ static __init void pvh_add_mem_mapping(s
         if ( !iomem_access_permitted(d, mfn + i, mfn + i) )
         {
             omfn = get_gfn_query_unlocked(d, gfn + i, &t);
-            guest_physmap_remove_page(d, gfn + i, mfn_x(omfn), PAGE_ORDER_4K);
+            if ( guest_physmap_remove_page(d, gfn + i, mfn_x(omfn),
+                                           PAGE_ORDER_4K) )
+                /* nothing, best effort only */;
             continue;
         }
 
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -671,8 +671,9 @@ bool_t is_ioreq_server_page(struct domai
 static void hvm_remove_ioreq_gmfn(
     struct domain *d, struct hvm_ioreq_page *iorp)
 {
-    guest_physmap_remove_page(d, iorp->gmfn, 
-                              page_to_mfn(iorp->page), 0);
+    if ( guest_physmap_remove_page(d, iorp->gmfn,
+                                   page_to_mfn(iorp->page), 0) )
+        domain_crash(d);
     clear_page(iorp->va);
 }
 
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4188,7 +4188,11 @@ static int replace_grant_p2m_mapping(
                 type, mfn_x(old_mfn), frame);
         return GNTST_general_error;
     }
-    guest_physmap_remove_page(d, gfn, frame, PAGE_ORDER_4K);
+    if ( guest_physmap_remove_page(d, gfn, frame, PAGE_ORDER_4K) )
+    {
+        put_gfn(d, gfn);
+        return GNTST_general_error;
+    }
 
     put_gfn(d, gfn);
     return GNTST_okay;
@@ -4712,7 +4716,7 @@ int xenmem_add_to_physmap_one(
     struct page_info *page = NULL;
     unsigned long gfn = 0; /* gcc ... */
     unsigned long prev_mfn, mfn = 0, old_gpfn;
-    int rc;
+    int rc = 0;
     p2m_type_t p2mt;
 
     switch ( space )
@@ -4786,25 +4790,30 @@ int xenmem_add_to_physmap_one(
     {
         if ( is_xen_heap_mfn(prev_mfn) )
             /* Xen heap frames are simply unhooked from this phys slot. */
-            guest_physmap_remove_page(d, gpfn, prev_mfn, PAGE_ORDER_4K);
+            rc = guest_physmap_remove_page(d, gpfn, prev_mfn, PAGE_ORDER_4K);
         else
             /* Normal domain memory is freed, to avoid leaking memory. */
-            guest_remove_page(d, gpfn);
+            rc = guest_remove_page(d, gpfn);
     }
     /* In the XENMAPSPACE_gmfn case we still hold a ref on the old page. */
     put_gfn(d, gpfn);
 
+    if ( rc )
+        goto put_both;
+
     /* Unmap from old location, if any. */
     old_gpfn = get_gpfn_from_mfn(mfn);
     ASSERT( old_gpfn != SHARED_M2P_ENTRY );
     if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range )
         ASSERT( old_gpfn == gfn );
     if ( old_gpfn != INVALID_M2P_ENTRY )
-        guest_physmap_remove_page(d, old_gpfn, mfn, PAGE_ORDER_4K);
+        rc = guest_physmap_remove_page(d, old_gpfn, mfn, PAGE_ORDER_4K);
 
     /* Map at new location. */
-    rc = guest_physmap_add_page(d, gpfn, mfn, PAGE_ORDER_4K);
+    if ( !rc )
+        rc = guest_physmap_add_page(d, gpfn, mfn, PAGE_ORDER_4K);
 
+ put_both:
     /* In the XENMAPSPACE_gmfn, we took a ref of the gfn at the top */
     if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range )
         put_gfn(d, gfn);
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2784,10 +2784,12 @@ int p2m_add_foreign(struct domain *tdom,
     {
         if ( is_xen_heap_mfn(prev_mfn) )
             /* Xen heap frames are simply unhooked from this phys slot */
-            guest_physmap_remove_page(tdom, gpfn, prev_mfn, 0);
+            rc = guest_physmap_remove_page(tdom, gpfn, prev_mfn, 0);
         else
             /* Normal domain memory is freed, to avoid leaking memory. */
-            guest_remove_page(tdom, gpfn);
+            rc = guest_remove_page(tdom, gpfn);
+        if ( rc )
+            goto put_both;
     }
     /*
      * Create the new mapping. Can't use guest_physmap_add_page() because it
@@ -2800,6 +2802,7 @@ int p2m_add_foreign(struct domain *tdom,
                  "gpfn:%lx mfn:%lx fgfn:%lx td:%d fd:%d\n",
                  gpfn, mfn, fgfn, tdom->domain_id, fdom->domain_id);
 
+ put_both:
     put_page(page);
 
     /*
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1755,6 +1755,7 @@ gnttab_transfer(
     for ( i = 0; i < count; i++ )
     {
         bool_t okay;
+        int rc;
 
         if (i && hypercall_preempt_check())
             return i;
@@ -1805,27 +1806,33 @@ gnttab_transfer(
             goto copyback;
         }
 
-        guest_physmap_remove_page(d, gop.mfn, mfn, 0);
+        rc = guest_physmap_remove_page(d, gop.mfn, mfn, 0);
         gnttab_flush_tlb(d);
+        if ( rc )
+        {
+            gdprintk(XENLOG_INFO,
+                     "gnttab_transfer: can't remove GFN %"PRI_xen_pfn" (MFN %lx)\n",
+                     gop.mfn, mfn);
+            gop.status = GNTST_general_error;
+            goto put_gfn_and_copyback;
+        }
 
         /* Find the target domain. */
         if ( unlikely((e = rcu_lock_domain_by_id(gop.domid)) == NULL) )
         {
-            put_gfn(d, gop.mfn);
             gdprintk(XENLOG_INFO, "gnttab_transfer: can't find domain %d\n",
                     gop.domid);
-            page->count_info &= ~(PGC_count_mask|PGC_allocated);
-            free_domheap_page(page);
             gop.status = GNTST_bad_domain;
-            goto copyback;
+            goto put_gfn_and_copyback;
         }
 
         if ( xsm_grant_transfer(XSM_HOOK, d, e) )
         {
-            put_gfn(d, gop.mfn);
             gop.status = GNTST_permission_denied;
         unlock_and_copyback:
             rcu_unlock_domain(e);
+        put_gfn_and_copyback:
+            put_gfn(d, gop.mfn);
             page->count_info &= ~(PGC_count_mask|PGC_allocated);
             free_domheap_page(page);
             goto copyback;
@@ -1874,12 +1981,8 @@ gnttab_transfer(
                          "Transferee (d%d) has no headroom (tot %u, max %u)\n",
                          e->domain_id, e->tot_pages, e->max_pages);
 
-            rcu_unlock_domain(e);
-            put_gfn(d, gop.mfn);
-            page->count_info &= ~(PGC_count_mask|PGC_allocated);
-            free_domheap_page(page);
             gop.status = GNTST_general_error;
-            goto copyback;
+            goto unlock_and_copyback;
         }
 
         /* Okay, add the page to 'e'. */
@@ -1908,13 +1911,8 @@ gnttab_transfer(
 
             if ( drop_dom_ref )
                 put_domain(e);
-            rcu_unlock_domain(e);
-
-            put_gfn(d, gop.mfn);
-            page->count_info &= ~(PGC_count_mask|PGC_allocated);
-            free_domheap_page(page);
             gop.status = GNTST_general_error;
-            goto copyback;
+            goto unlock_and_copyback;
         }
 
         page_list_add_tail(page, &e->page_list);
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -246,8 +246,12 @@ int guest_remove_page(struct domain *d,
     mfn = mfn_x(get_gfn_query(d, gmfn, &p2mt)); 
     if ( unlikely(p2m_is_paging(p2mt)) )
     {
-        guest_physmap_remove_page(d, gmfn, mfn, 0);
+        rc = guest_physmap_remove_page(d, gmfn, mfn, 0);
         put_gfn(d, gmfn);
+
+        if ( rc )
+            return rc;
+
         /* 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 )
@@ -311,18 +315,18 @@ int guest_remove_page(struct domain *d,
         return -ENXIO;
     }
 
-    if ( test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
+    rc = guest_physmap_remove_page(d, gmfn, mfn, 0);
+
+    if ( !rc && test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
         put_page_and_type(page);
             
-    if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
+    if ( !rc && test_and_clear_bit(_PGC_allocated, &page->count_info) )
         put_page(page);
 
-    guest_physmap_remove_page(d, gmfn, mfn, 0);
-
     put_page(page);
     put_gfn(d, gmfn);
 
-    return 0;
+    return rc;
 }
 
 static void decrease_reservation(struct memop_args *a)
@@ -564,7 +568,8 @@ static long memory_exchange(XEN_GUEST_HA
             gfn = mfn_to_gmfn(d, mfn);
             /* Pages were unshared above */
             BUG_ON(SHARED_M2P(gfn));
-            guest_physmap_remove_page(d, gfn, mfn, 0);
+            if ( guest_physmap_remove_page(d, gfn, mfn, 0) )
+                domain_crash(d);
             put_page(page);
         }
 
@@ -1097,7 +1102,7 @@ long do_memory_op(unsigned long cmd, XEN
         page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC);
         if ( page )
         {
-            guest_physmap_remove_page(d, xrfp.gpfn, page_to_mfn(page), 0);
+            rc = guest_physmap_remove_page(d, xrfp.gpfn, page_to_mfn(page), 0);
             put_page(page);
         }
         else
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2787,9 +2787,7 @@ static int arm_smmu_unmap_page(struct do
 	if ( !is_domain_direct_mapped(d) )
 		return -EINVAL;
 
-	guest_physmap_remove_page(d, gfn, gfn, 0);
-
-	return 0;
+	return guest_physmap_remove_page(d, gfn, gfn, 0);
 }
 
 static const struct iommu_ops arm_smmu_iommu_ops = {
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -173,10 +173,6 @@ static inline int guest_physmap_add_page
     return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw);
 }
 
-void guest_physmap_remove_page(struct domain *d,
-                               unsigned long gpfn,
-                               unsigned long mfn, unsigned int page_order);
-
 unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn);
 
 /*
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -536,11 +536,6 @@ static inline int guest_physmap_add_page
     return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw);
 }
 
-/* Remove a page from a domain's p2m table */
-int guest_physmap_remove_page(struct domain *d,
-                              unsigned long gfn,
-                              unsigned long mfn, unsigned int page_order);
-
 /* Set a p2m range as populate-on-demand */
 int guest_physmap_mark_populate_on_demand(struct domain *d, unsigned long gfn,
                                           unsigned int order);
--- a/xen/include/xen/p2m-common.h
+++ b/xen/include/xen/p2m-common.h
@@ -1,6 +1,7 @@
 #ifndef _XEN_P2M_COMMON_H
 #define _XEN_P2M_COMMON_H
 
+#include <xen/mm.h>
 #include <public/vm_event.h>
 
 /*
@@ -33,6 +34,11 @@ typedef enum {
     /* NOTE: Assumed to be only 4 bits right now on x86. */
 } p2m_access_t;
 
+/* Remove a page from a domain's p2m table */
+int __must_check
+guest_physmap_remove_page(struct domain *d, unsigned long gfn,
+                          unsigned long mfn, unsigned int page_order);
+
 /* Map MMIO regions in the p2m: start_gfn and nr describe the range in
  *  * the guest physical address space to map, starting from the machine
  *   * frame number mfn. */
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -448,7 +448,7 @@ int xenmem_add_to_physmap_one(struct dom
                               unsigned long idx, xen_pfn_t gpfn);
 
 /* Returns 0 on success, or negative on error. */
-int guest_remove_page(struct domain *d, unsigned long gmfn);
+int __must_check guest_remove_page(struct domain *d, unsigned long gmfn);
 
 #define RAM_TYPE_CONVENTIONAL 0x00000001
 #define RAM_TYPE_RESERVED     0x00000002

[-- Attachment #8: xsa222-2-4.7.patch --]
[-- Type: application/octet-stream, Size: 14631 bytes --]

From: Jan Beulich <jbeulich@suse.com>
Subject: guest_physmap_remove_page() needs its return value checked

Callers, namely such subsequently freeing the page, must not blindly
assume success - the function may namely fail when needing to shatter a
super page, but there not being memory available for the then needed
intermediate page table.

As it happens, guest_remove_page() callers now also all check the
return value.

Furthermore a missed put_gfn() on an error path in gnttab_transfer() is
also being taken care of.

This is part of XSA-222.

Reported-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1299,13 +1299,14 @@ int replace_grant_host_mapping(unsigned
 {
     unsigned long gfn = (unsigned long)(addr >> PAGE_SHIFT);
     struct domain *d = current->domain;
+    int rc;
 
     if ( new_addr != 0 || (flags & GNTMAP_contains_pte) )
         return GNTST_general_error;
 
-    guest_physmap_remove_page(d, gfn, mfn, 0);
+    rc = guest_physmap_remove_page(d, gfn, mfn, 0);
 
-    return GNTST_okay;
+    return rc ? GNTST_general_error : GNTST_okay;
 }
 
 int is_iomem_page(unsigned long mfn)
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1313,15 +1313,14 @@ int guest_physmap_add_entry(struct domai
                              d->arch.p2m.default_access);
 }
 
-void guest_physmap_remove_page(struct domain *d,
-                               unsigned long gpfn,
-                               unsigned long mfn, unsigned int page_order)
+int guest_physmap_remove_page(struct domain *d, unsigned long gfn,
+                              unsigned long mfn, unsigned int page_order)
 {
-    apply_p2m_changes(d, REMOVE,
-                      pfn_to_paddr(gpfn),
-                      pfn_to_paddr(gpfn + (1<<page_order)),
-                      pfn_to_paddr(mfn), MATTR_MEM, 0, p2m_invalid,
-                      d->arch.p2m.default_access);
+    return apply_p2m_changes(d, REMOVE,
+                             pfn_to_paddr(gfn),
+                             pfn_to_paddr(gfn + (1 << page_order)),
+                             pfn_to_paddr(mfn), MATTR_MEM, 0, p2m_invalid,
+                             d->arch.p2m.default_access);
 }
 
 int p2m_alloc_table(struct domain *d)
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -802,7 +802,15 @@ int arch_domain_soft_reset(struct domain
         ret = -ENOMEM;
         goto exit_put_gfn;
     }
-    guest_physmap_remove_page(d, gfn, mfn, PAGE_ORDER_4K);
+
+    ret = guest_physmap_remove_page(d, gfn, mfn, PAGE_ORDER_4K);
+    if ( ret )
+    {
+        printk(XENLOG_G_ERR "Failed to remove Dom%d's shared_info frame %lx\n",
+               d->domain_id, gfn);
+        free_domheap_page(new_page);
+        goto exit_put_gfn;
+    }
 
     ret = guest_physmap_add_page(d, gfn, page_to_mfn(new_page), PAGE_ORDER_4K);
     if ( ret )
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -427,7 +427,9 @@ static __init void pvh_add_mem_mapping(s
         if ( !iomem_access_permitted(d, mfn + i, mfn + i) )
         {
             omfn = get_gfn_query_unlocked(d, gfn + i, &t);
-            guest_physmap_remove_page(d, gfn + i, mfn_x(omfn), PAGE_ORDER_4K);
+            if ( guest_physmap_remove_page(d, gfn + i, mfn_x(omfn),
+                                           PAGE_ORDER_4K) )
+                /* nothing, best effort only */;
             continue;
         }
 
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -267,8 +267,9 @@ bool_t is_ioreq_server_page(struct domai
 static void hvm_remove_ioreq_gmfn(
     struct domain *d, struct hvm_ioreq_page *iorp)
 {
-    guest_physmap_remove_page(d, iorp->gmfn,
-                              page_to_mfn(iorp->page), 0);
+    if ( guest_physmap_remove_page(d, iorp->gmfn,
+                                   page_to_mfn(iorp->page), 0) )
+        domain_crash(d);
     clear_page(iorp->va);
 }
 
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4271,7 +4271,11 @@ static int replace_grant_p2m_mapping(
                 type, mfn_x(old_mfn), frame);
         return GNTST_general_error;
     }
-    guest_physmap_remove_page(d, gfn, frame, PAGE_ORDER_4K);
+    if ( guest_physmap_remove_page(d, gfn, frame, PAGE_ORDER_4K) )
+    {
+        put_gfn(d, gfn);
+        return GNTST_general_error;
+    }
 
     put_gfn(d, gfn);
     return GNTST_okay;
@@ -4793,7 +4797,7 @@ int xenmem_add_to_physmap_one(
     struct page_info *page = NULL;
     unsigned long gfn = 0; /* gcc ... */
     unsigned long prev_mfn, mfn = 0, old_gpfn;
-    int rc;
+    int rc = 0;
     p2m_type_t p2mt;
 
     switch ( space )
@@ -4867,25 +4871,30 @@ int xenmem_add_to_physmap_one(
     {
         if ( is_xen_heap_mfn(prev_mfn) )
             /* Xen heap frames are simply unhooked from this phys slot. */
-            guest_physmap_remove_page(d, gpfn, prev_mfn, PAGE_ORDER_4K);
+            rc = guest_physmap_remove_page(d, gpfn, prev_mfn, PAGE_ORDER_4K);
         else
             /* Normal domain memory is freed, to avoid leaking memory. */
-            guest_remove_page(d, gpfn);
+            rc = guest_remove_page(d, gpfn);
     }
     /* In the XENMAPSPACE_gmfn case we still hold a ref on the old page. */
     put_gfn(d, gpfn);
 
+    if ( rc )
+        goto put_both;
+
     /* Unmap from old location, if any. */
     old_gpfn = get_gpfn_from_mfn(mfn);
     ASSERT( old_gpfn != SHARED_M2P_ENTRY );
     if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range )
         ASSERT( old_gpfn == gfn );
     if ( old_gpfn != INVALID_M2P_ENTRY )
-        guest_physmap_remove_page(d, old_gpfn, mfn, PAGE_ORDER_4K);
+        rc = guest_physmap_remove_page(d, old_gpfn, mfn, PAGE_ORDER_4K);
 
     /* Map at new location. */
-    rc = guest_physmap_add_page(d, gpfn, mfn, PAGE_ORDER_4K);
+    if ( !rc )
+        rc = guest_physmap_add_page(d, gpfn, mfn, PAGE_ORDER_4K);
 
+ put_both:
     /* In the XENMAPSPACE_gmfn, we took a ref of the gfn at the top */
     if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range )
         put_gfn(d, gfn);
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2837,10 +2837,12 @@ int p2m_add_foreign(struct domain *tdom,
     {
         if ( is_xen_heap_mfn(prev_mfn) )
             /* Xen heap frames are simply unhooked from this phys slot */
-            guest_physmap_remove_page(tdom, gpfn, prev_mfn, 0);
+            rc = guest_physmap_remove_page(tdom, gpfn, prev_mfn, 0);
         else
             /* Normal domain memory is freed, to avoid leaking memory. */
-            guest_remove_page(tdom, gpfn);
+            rc = guest_remove_page(tdom, gpfn);
+        if ( rc )
+            goto put_both;
     }
     /*
      * Create the new mapping. Can't use guest_physmap_add_page() because it
@@ -2853,6 +2855,7 @@ int p2m_add_foreign(struct domain *tdom,
                  "gpfn:%lx mfn:%lx fgfn:%lx td:%d fd:%d\n",
                  gpfn, mfn, fgfn, tdom->domain_id, fdom->domain_id);
 
+ put_both:
     put_page(page);
 
     /*
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1768,6 +1768,7 @@ gnttab_transfer(
     for ( i = 0; i < count; i++ )
     {
         bool_t okay;
+        int rc;
 
         if (i && hypercall_preempt_check())
             return i;
@@ -1818,27 +1819,33 @@ gnttab_transfer(
             goto copyback;
         }
 
-        guest_physmap_remove_page(d, gop.mfn, mfn, 0);
+        rc = guest_physmap_remove_page(d, gop.mfn, mfn, 0);
         gnttab_flush_tlb(d);
+        if ( rc )
+        {
+            gdprintk(XENLOG_INFO,
+                     "gnttab_transfer: can't remove GFN %"PRI_xen_pfn" (MFN %lx)\n",
+                     gop.mfn, mfn);
+            gop.status = GNTST_general_error;
+            goto put_gfn_and_copyback;
+        }
 
         /* Find the target domain. */
         if ( unlikely((e = rcu_lock_domain_by_id(gop.domid)) == NULL) )
         {
-            put_gfn(d, gop.mfn);
             gdprintk(XENLOG_INFO, "gnttab_transfer: can't find domain %d\n",
                     gop.domid);
-            page->count_info &= ~(PGC_count_mask|PGC_allocated);
-            free_domheap_page(page);
             gop.status = GNTST_bad_domain;
-            goto copyback;
+            goto put_gfn_and_copyback;
         }
 
         if ( xsm_grant_transfer(XSM_HOOK, d, e) )
         {
-            put_gfn(d, gop.mfn);
             gop.status = GNTST_permission_denied;
         unlock_and_copyback:
             rcu_unlock_domain(e);
+        put_gfn_and_copyback:
+            put_gfn(d, gop.mfn);
             page->count_info &= ~(PGC_count_mask|PGC_allocated);
             free_domheap_page(page);
             goto copyback;
@@ -1887,12 +1894,8 @@ gnttab_transfer(
                          "Transferee (d%d) has no headroom (tot %u, max %u)\n",
                          e->domain_id, e->tot_pages, e->max_pages);
 
-            rcu_unlock_domain(e);
-            put_gfn(d, gop.mfn);
-            page->count_info &= ~(PGC_count_mask|PGC_allocated);
-            free_domheap_page(page);
             gop.status = GNTST_general_error;
-            goto copyback;
+            goto unlock_and_copyback;
         }
 
         /* Okay, add the page to 'e'. */
@@ -1921,13 +1924,8 @@ gnttab_transfer(
 
             if ( drop_dom_ref )
                 put_domain(e);
-            rcu_unlock_domain(e);
-
-            put_gfn(d, gop.mfn);
-            page->count_info &= ~(PGC_count_mask|PGC_allocated);
-            free_domheap_page(page);
             gop.status = GNTST_general_error;
-            goto copyback;
+            goto unlock_and_copyback;
         }
 
         page_list_add_tail(page, &e->page_list);
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -250,8 +250,12 @@ int guest_remove_page(struct domain *d,
     mfn = mfn_x(get_gfn_query(d, gmfn, &p2mt)); 
     if ( unlikely(p2m_is_paging(p2mt)) )
     {
-        guest_physmap_remove_page(d, gmfn, mfn, 0);
+        rc = guest_physmap_remove_page(d, gmfn, mfn, 0);
         put_gfn(d, gmfn);
+
+        if ( rc )
+            return rc;
+
         /* 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 )
@@ -315,7 +319,9 @@ int guest_remove_page(struct domain *d,
         return -ENXIO;
     }
 
-    if ( test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
+    rc = guest_physmap_remove_page(d, gmfn, mfn, 0);
+
+    if ( !rc && test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
         put_page_and_type(page);
 
     /*
@@ -326,16 +332,14 @@ int guest_remove_page(struct domain *d,
      * For this purpose (and to match populate_physmap() behavior), the page
      * is kept allocated.
      */
-    if ( !is_domain_direct_mapped(d) &&
+    if ( !rc && !is_domain_direct_mapped(d) &&
          test_and_clear_bit(_PGC_allocated, &page->count_info) )
         put_page(page);
 
-    guest_physmap_remove_page(d, gmfn, mfn, 0);
-
     put_page(page);
     put_gfn(d, gmfn);
 
-    return 0;
+    return rc;
 }
 
 static void decrease_reservation(struct memop_args *a)
@@ -570,7 +574,8 @@ static long memory_exchange(XEN_GUEST_HA
             gfn = mfn_to_gmfn(d, mfn);
             /* Pages were unshared above */
             BUG_ON(SHARED_M2P(gfn));
-            guest_physmap_remove_page(d, gfn, mfn, 0);
+            if ( guest_physmap_remove_page(d, gfn, mfn, 0) )
+                domain_crash(d);
             put_page(page);
         }
 
@@ -1120,7 +1125,7 @@ long do_memory_op(unsigned long cmd, XEN
         page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC);
         if ( page )
         {
-            guest_physmap_remove_page(d, xrfp.gpfn, page_to_mfn(page), 0);
+            rc = guest_physmap_remove_page(d, xrfp.gpfn, page_to_mfn(page), 0);
             put_page(page);
         }
         else
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2783,9 +2783,7 @@ static int arm_smmu_unmap_page(struct do
 	if ( !is_domain_direct_mapped(d) )
 		return -EINVAL;
 
-	guest_physmap_remove_page(d, gfn, gfn, 0);
-
-	return 0;
+	return guest_physmap_remove_page(d, gfn, gfn, 0);
 }
 
 static const struct iommu_ops arm_smmu_iommu_ops = {
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -177,10 +177,6 @@ static inline int guest_physmap_add_page
     return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw);
 }
 
-void guest_physmap_remove_page(struct domain *d,
-                               unsigned long gpfn,
-                               unsigned long mfn, unsigned int page_order);
-
 unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn);
 
 /*
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -558,11 +558,6 @@ static inline int guest_physmap_add_page
     return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw);
 }
 
-/* Remove a page from a domain's p2m table */
-int guest_physmap_remove_page(struct domain *d,
-                              unsigned long gfn,
-                              unsigned long mfn, unsigned int page_order);
-
 /* Set a p2m range as populate-on-demand */
 int guest_physmap_mark_populate_on_demand(struct domain *d, unsigned long gfn,
                                           unsigned int order);
--- a/xen/include/xen/p2m-common.h
+++ b/xen/include/xen/p2m-common.h
@@ -1,6 +1,7 @@
 #ifndef _XEN_P2M_COMMON_H
 #define _XEN_P2M_COMMON_H
 
+#include <xen/mm.h>
 #include <public/vm_event.h>
 
 /*
@@ -33,6 +34,11 @@ typedef enum {
     /* NOTE: Assumed to be only 4 bits right now on x86. */
 } p2m_access_t;
 
+/* Remove a page from a domain's p2m table */
+int __must_check
+guest_physmap_remove_page(struct domain *d, unsigned long gfn,
+                          unsigned long mfn, unsigned int page_order);
+
 /* Map MMIO regions in the p2m: start_gfn and nr describe the range in
  *  * the guest physical address space to map, starting from the machine
  *   * frame number mfn. */
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -510,7 +510,7 @@ int xenmem_add_to_physmap_one(struct dom
                               unsigned long idx, xen_pfn_t gpfn);
 
 /* Returns 0 on success, or negative on error. */
-int guest_remove_page(struct domain *d, unsigned long gmfn);
+int __must_check guest_remove_page(struct domain *d, unsigned long gmfn);
 
 #define RAM_TYPE_CONVENTIONAL 0x00000001
 #define RAM_TYPE_RESERVED     0x00000002

[-- Attachment #9: xsa222-2-4.8.patch --]
[-- Type: application/octet-stream, Size: 14461 bytes --]

From: Jan Beulich <jbeulich@suse.com>
Subject: guest_physmap_remove_page() needs its return value checked

Callers, namely such subsequently freeing the page, must not blindly
assume success - the function may namely fail when needing to shatter a
super page, but there not being memory available for the then needed
intermediate page table.

As it happens, guest_remove_page() callers now also all check the
return value.

Furthermore a missed put_gfn() on an error path in gnttab_transfer() is
also being taken care of.

This is part of XSA-222.

Reported-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1340,13 +1340,14 @@ int replace_grant_host_mapping(unsigned
 {
     gfn_t gfn = _gfn(addr >> PAGE_SHIFT);
     struct domain *d = current->domain;
+    int rc;
 
     if ( new_addr != 0 || (flags & GNTMAP_contains_pte) )
         return GNTST_general_error;
 
-    guest_physmap_remove_page(d, gfn, _mfn(mfn), 0);
+    rc = guest_physmap_remove_page(d, gfn, _mfn(mfn), 0);
 
-    return GNTST_okay;
+    return rc ? GNTST_general_error : GNTST_okay;
 }
 
 int is_iomem_page(unsigned long mfn)
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1211,11 +1211,10 @@ int guest_physmap_add_entry(struct domai
     return p2m_insert_mapping(d, gfn, (1 << page_order), mfn, t);
 }
 
-void guest_physmap_remove_page(struct domain *d,
-                               gfn_t gfn,
-                               mfn_t mfn, unsigned int page_order)
+int guest_physmap_remove_page(struct domain *d, gfn_t gfn, mfn_t mfn,
+                              unsigned int page_order)
 {
-    p2m_remove_mapping(d, gfn, (1 << page_order), mfn);
+    return p2m_remove_mapping(d, gfn, (1 << page_order), mfn);
 }
 
 static int p2m_alloc_table(struct domain *d)
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -808,7 +808,15 @@ int arch_domain_soft_reset(struct domain
         ret = -ENOMEM;
         goto exit_put_gfn;
     }
-    guest_physmap_remove_page(d, _gfn(gfn), _mfn(mfn), PAGE_ORDER_4K);
+
+    ret = guest_physmap_remove_page(d, _gfn(gfn), _mfn(mfn), PAGE_ORDER_4K);
+    if ( ret )
+    {
+        printk(XENLOG_G_ERR "Failed to remove Dom%d's shared_info frame %lx\n",
+               d->domain_id, gfn);
+        free_domheap_page(new_page);
+        goto exit_put_gfn;
+    }
 
     ret = guest_physmap_add_page(d, _gfn(gfn), _mfn(page_to_mfn(new_page)),
                                  PAGE_ORDER_4K);
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -427,7 +427,9 @@ static __init void pvh_add_mem_mapping(s
         if ( !iomem_access_permitted(d, mfn + i, mfn + i) )
         {
             omfn = get_gfn_query_unlocked(d, gfn + i, &t);
-            guest_physmap_remove_page(d, _gfn(gfn + i), omfn, PAGE_ORDER_4K);
+            if ( guest_physmap_remove_page(d, _gfn(gfn + i), omfn,
+                                           PAGE_ORDER_4K) )
+                /* nothing, best effort only */;
             continue;
         }
 
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -267,8 +267,9 @@ bool_t is_ioreq_server_page(struct domai
 static void hvm_remove_ioreq_gmfn(
     struct domain *d, struct hvm_ioreq_page *iorp)
 {
-    guest_physmap_remove_page(d, _gfn(iorp->gmfn),
-                              _mfn(page_to_mfn(iorp->page)), 0);
+    if ( guest_physmap_remove_page(d, _gfn(iorp->gmfn),
+                                   _mfn(page_to_mfn(iorp->page)), 0) )
+        domain_crash(d);
     clear_page(iorp->va);
 }
 
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4276,7 +4276,11 @@ static int replace_grant_p2m_mapping(
                 type, mfn_x(old_mfn), frame);
         return GNTST_general_error;
     }
-    guest_physmap_remove_page(d, _gfn(gfn), _mfn(frame), PAGE_ORDER_4K);
+    if ( guest_physmap_remove_page(d, _gfn(gfn), _mfn(frame), PAGE_ORDER_4K) )
+    {
+        put_gfn(d, gfn);
+        return GNTST_general_error;
+    }
 
     put_gfn(d, gfn);
     return GNTST_okay;
@@ -4798,7 +4802,7 @@ int xenmem_add_to_physmap_one(
     struct page_info *page = NULL;
     unsigned long gfn = 0; /* gcc ... */
     unsigned long prev_mfn, mfn = 0, old_gpfn;
-    int rc;
+    int rc = 0;
     p2m_type_t p2mt;
 
     switch ( space )
@@ -4872,25 +4876,30 @@ int xenmem_add_to_physmap_one(
     {
         if ( is_xen_heap_mfn(prev_mfn) )
             /* Xen heap frames are simply unhooked from this phys slot. */
-            guest_physmap_remove_page(d, gpfn, _mfn(prev_mfn), PAGE_ORDER_4K);
+            rc = guest_physmap_remove_page(d, gpfn, _mfn(prev_mfn), PAGE_ORDER_4K);
         else
             /* Normal domain memory is freed, to avoid leaking memory. */
-            guest_remove_page(d, gfn_x(gpfn));
+            rc = guest_remove_page(d, gfn_x(gpfn));
     }
     /* In the XENMAPSPACE_gmfn case we still hold a ref on the old page. */
     put_gfn(d, gfn_x(gpfn));
 
+    if ( rc )
+        goto put_both;
+
     /* Unmap from old location, if any. */
     old_gpfn = get_gpfn_from_mfn(mfn);
     ASSERT( old_gpfn != SHARED_M2P_ENTRY );
     if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range )
         ASSERT( old_gpfn == gfn );
     if ( old_gpfn != INVALID_M2P_ENTRY )
-        guest_physmap_remove_page(d, _gfn(old_gpfn), _mfn(mfn), PAGE_ORDER_4K);
+        rc = guest_physmap_remove_page(d, _gfn(old_gpfn), _mfn(mfn), PAGE_ORDER_4K);
 
     /* Map at new location. */
-    rc = guest_physmap_add_page(d, gpfn, _mfn(mfn), PAGE_ORDER_4K);
+    if ( !rc )
+        rc = guest_physmap_add_page(d, gpfn, _mfn(mfn), PAGE_ORDER_4K);
 
+ put_both:
     /* In the XENMAPSPACE_gmfn, we took a ref of the gfn at the top */
     if ( space == XENMAPSPACE_gmfn || space == XENMAPSPACE_gmfn_range )
         put_gfn(d, gfn);
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2925,10 +2925,12 @@ int p2m_add_foreign(struct domain *tdom,
     {
         if ( is_xen_heap_mfn(mfn_x(prev_mfn)) )
             /* Xen heap frames are simply unhooked from this phys slot */
-            guest_physmap_remove_page(tdom, _gfn(gpfn), prev_mfn, 0);
+            rc = guest_physmap_remove_page(tdom, _gfn(gpfn), prev_mfn, 0);
         else
             /* Normal domain memory is freed, to avoid leaking memory. */
-            guest_remove_page(tdom, gpfn);
+            rc = guest_remove_page(tdom, gpfn);
+        if ( rc )
+            goto put_both;
     }
     /*
      * Create the new mapping. Can't use guest_physmap_add_page() because it
@@ -2941,6 +2943,7 @@ int p2m_add_foreign(struct domain *tdom,
                  "gpfn:%lx mfn:%lx fgfn:%lx td:%d fd:%d\n",
                  gpfn, mfn_x(mfn), fgfn, tdom->domain_id, fdom->domain_id);
 
+ put_both:
     put_page(page);
 
     /*
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1768,6 +1768,7 @@ gnttab_transfer(
     for ( i = 0; i < count; i++ )
     {
         bool_t okay;
+        int rc;
 
         if (i && hypercall_preempt_check())
             return i;
@@ -1818,27 +1819,33 @@ gnttab_transfer(
             goto copyback;
         }
 
-        guest_physmap_remove_page(d, _gfn(gop.mfn), _mfn(mfn), 0);
+        rc = guest_physmap_remove_page(d, _gfn(gop.mfn), _mfn(mfn), 0);
         gnttab_flush_tlb(d);
+        if ( rc )
+        {
+            gdprintk(XENLOG_INFO,
+                     "gnttab_transfer: can't remove GFN %"PRI_xen_pfn" (MFN %lx)\n",
+                     gop.mfn, mfn);
+            gop.status = GNTST_general_error;
+            goto put_gfn_and_copyback;
+        }
 
         /* Find the target domain. */
         if ( unlikely((e = rcu_lock_domain_by_id(gop.domid)) == NULL) )
         {
-            put_gfn(d, gop.mfn);
             gdprintk(XENLOG_INFO, "gnttab_transfer: can't find domain %d\n",
                     gop.domid);
-            page->count_info &= ~(PGC_count_mask|PGC_allocated);
-            free_domheap_page(page);
             gop.status = GNTST_bad_domain;
-            goto copyback;
+            goto put_gfn_and_copyback;
         }
 
         if ( xsm_grant_transfer(XSM_HOOK, d, e) )
         {
-            put_gfn(d, gop.mfn);
             gop.status = GNTST_permission_denied;
         unlock_and_copyback:
             rcu_unlock_domain(e);
+        put_gfn_and_copyback:
+            put_gfn(d, gop.mfn);
             page->count_info &= ~(PGC_count_mask|PGC_allocated);
             free_domheap_page(page);
             goto copyback;
@@ -1887,12 +1894,8 @@ gnttab_transfer(
                          "Transferee (d%d) has no headroom (tot %u, max %u)\n",
                          e->domain_id, e->tot_pages, e->max_pages);
 
-            rcu_unlock_domain(e);
-            put_gfn(d, gop.mfn);
-            page->count_info &= ~(PGC_count_mask|PGC_allocated);
-            free_domheap_page(page);
             gop.status = GNTST_general_error;
-            goto copyback;
+            goto unlock_and_copyback;
         }
 
         /* Okay, add the page to 'e'. */
@@ -1921,13 +1924,8 @@ gnttab_transfer(
 
             if ( drop_dom_ref )
                 put_domain(e);
-            rcu_unlock_domain(e);
-
-            put_gfn(d, gop.mfn);
-            page->count_info &= ~(PGC_count_mask|PGC_allocated);
-            free_domheap_page(page);
             gop.status = GNTST_general_error;
-            goto copyback;
+            goto unlock_and_copyback;
         }
 
         page_list_add_tail(page, &e->page_list);
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -272,8 +272,12 @@ int guest_remove_page(struct domain *d,
     mfn = get_gfn_query(d, gmfn, &p2mt);
     if ( unlikely(p2m_is_paging(p2mt)) )
     {
-        guest_physmap_remove_page(d, _gfn(gmfn), mfn, 0);
+        rc = guest_physmap_remove_page(d, _gfn(gmfn), mfn, 0);
         put_gfn(d, gmfn);
+
+        if ( rc )
+            return rc;
+
         /* 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 )
@@ -337,7 +341,9 @@ int guest_remove_page(struct domain *d,
         return -ENXIO;
     }
 
-    if ( test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
+    rc = guest_physmap_remove_page(d, _gfn(gmfn), mfn, 0);
+
+    if ( !rc && test_and_clear_bit(_PGT_pinned, &page->u.inuse.type_info) )
         put_page_and_type(page);
 
     /*
@@ -348,16 +354,14 @@ int guest_remove_page(struct domain *d,
      * For this purpose (and to match populate_physmap() behavior), the page
      * is kept allocated.
      */
-    if ( !is_domain_direct_mapped(d) &&
+    if ( !rc && !is_domain_direct_mapped(d) &&
          test_and_clear_bit(_PGC_allocated, &page->count_info) )
         put_page(page);
 
-    guest_physmap_remove_page(d, _gfn(gmfn), mfn, 0);
-
     put_page(page);
     put_gfn(d, gmfn);
 
-    return 0;
+    return rc;
 }
 
 static void decrease_reservation(struct memop_args *a)
@@ -592,7 +596,8 @@ static long memory_exchange(XEN_GUEST_HA
             gfn = mfn_to_gmfn(d, mfn);
             /* Pages were unshared above */
             BUG_ON(SHARED_M2P(gfn));
-            guest_physmap_remove_page(d, _gfn(gfn), _mfn(mfn), 0);
+            if ( guest_physmap_remove_page(d, _gfn(gfn), _mfn(mfn), 0) )
+                domain_crash(d);
             put_page(page);
         }
 
@@ -1151,8 +1156,8 @@ long do_memory_op(unsigned long cmd, XEN
         page = get_page_from_gfn(d, xrfp.gpfn, NULL, P2M_ALLOC);
         if ( page )
         {
-            guest_physmap_remove_page(d, _gfn(xrfp.gpfn),
-                                      _mfn(page_to_mfn(page)), 0);
+            rc = guest_physmap_remove_page(d, _gfn(xrfp.gpfn),
+                                           _mfn(page_to_mfn(page)), 0);
             put_page(page);
         }
         else
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -2786,9 +2786,7 @@ static int __must_check arm_smmu_unmap_p
 	if ( !is_domain_direct_mapped(d) )
 		return -EINVAL;
 
-	guest_physmap_remove_page(d, _gfn(gfn), _mfn(gfn), 0);
-
-	return 0;
+	return guest_physmap_remove_page(d, _gfn(gfn), _mfn(gfn), 0);
 }
 
 static const struct iommu_ops arm_smmu_iommu_ops = {
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -268,10 +268,6 @@ static inline int guest_physmap_add_page
     return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw);
 }
 
-void guest_physmap_remove_page(struct domain *d,
-                               gfn_t gfn,
-                               mfn_t mfn, unsigned int page_order);
-
 mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn);
 
 /*
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -561,10 +561,6 @@ static inline int guest_physmap_add_page
     return guest_physmap_add_entry(d, gfn, mfn, page_order, p2m_ram_rw);
 }
 
-/* Remove a page from a domain's p2m table */
-int guest_physmap_remove_page(struct domain *d,
-                              gfn_t gfn, mfn_t mfn, unsigned int page_order);
-
 /* Set a p2m range as populate-on-demand */
 int guest_physmap_mark_populate_on_demand(struct domain *d, unsigned long gfn,
                                           unsigned int order);
--- a/xen/include/xen/p2m-common.h
+++ b/xen/include/xen/p2m-common.h
@@ -1,6 +1,7 @@
 #ifndef _XEN_P2M_COMMON_H
 #define _XEN_P2M_COMMON_H
 
+#include <xen/mm.h>
 #include <public/vm_event.h>
 
 /*
@@ -33,6 +34,11 @@ typedef enum {
     /* NOTE: Assumed to be only 4 bits right now on x86. */
 } p2m_access_t;
 
+/* Remove a page from a domain's p2m table */
+int __must_check
+guest_physmap_remove_page(struct domain *d, gfn_t gfn, mfn_t mfn,
+                          unsigned int page_order);
+
 /* Map MMIO regions in the p2m: start_gfn and nr describe the range in
  *  * the guest physical address space to map, starting from the machine
  *   * frame number mfn. */
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -554,7 +554,7 @@ int xenmem_add_to_physmap_one(struct dom
                               unsigned long idx, gfn_t gfn);
 
 /* Returns 0 on success, or negative on error. */
-int guest_remove_page(struct domain *d, unsigned long gmfn);
+int __must_check guest_remove_page(struct domain *d, unsigned long gmfn);
 
 #define RAM_TYPE_CONVENTIONAL 0x00000001
 #define RAM_TYPE_RESERVED     0x00000002

[-- Attachment #10: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2017-06-20 12:00 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-20 12:00 Xen Security Advisory 222 - stale P2M mappings due to insufficient error checking Xen.org security team

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).