|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [xen staging-4.20] gnttab: split gnttab_map_frame()
commit 51e09058d589914ee46b8e996bd6e5d322c5cf32
Author: Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Tue Apr 28 13:41:28 2026 +0100
Commit: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Tue Apr 28 13:43:55 2026 +0100
gnttab: split gnttab_map_frame()
If a domain tries to map status frames in parallel to switching grant
table version from 2 to 1, the mapping operation may put in place P2M
entries referencing MFNs which gnttab_unpopulate_status_frames() is in the
process of freeing.
Ideally we would refcount pages when entered into P2M tables, but that's a
significant change. Extend the grant-table-locked region instead in
xenmem_add_to_physmap_one() (being the sole caller of gnttab_map_frame()),
such that a race with gnttab_unpopulate_status_frames() is no longer
possible.
This is XSA-486 / CVE-2026-23558.
Fixes: 5ce8fafa947c ("Dynamic grant-table sizing")
Fixes: a98dc13703e0 ("Introduce a grant_entry_v2 structure")
Reported-by: Rafal Wojtczuk <rafal.wojtczuk@xxxxxxxxxx>
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
(cherry picked from commit 02708d3b51b04aeeaa4b127637d7fec8d8c5b1d8)
---
xen/arch/arm/mm.c | 19 +++++++++++++++----
xen/arch/x86/mm/p2m.c | 25 ++++++++++++++++---------
xen/common/grant_table.c | 13 +++++++++++--
xen/include/xen/grant_table.h | 15 +++++++++++----
4 files changed, 53 insertions(+), 19 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index a56e20ba2b..4b391ac612 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -114,12 +114,10 @@ int xenmem_add_to_physmap_one(
switch ( space )
{
case XENMAPSPACE_grant_table:
- rc = gnttab_map_frame(d, idx, gfn, &mfn);
+ rc = gnttab_map_frame_begin(d, idx, gfn, &mfn);
if ( rc )
return rc;
- /* Need to take care of the reference obtained in gnttab_map_frame().
*/
- page = mfn_to_page(mfn);
t = p2m_ram_rw;
break;
@@ -221,10 +219,23 @@ int xenmem_add_to_physmap_one(
* to drop the reference we took earlier. In all other cases we need to
* drop any reference we took earlier (perhaps indirectly).
*/
- if ( space == XENMAPSPACE_gmfn_foreign ? rc : page != NULL )
+ switch ( space )
{
+ default:
+ if ( page )
+ put_page(page);
+ break;
+
+ case XENMAPSPACE_grant_table:
+ gnttab_map_frame_end(d, mfn);
+ break;
+
+ case XENMAPSPACE_gmfn_foreign:
+ if ( !rc )
+ break;
ASSERT(page != NULL);
put_page(page);
+ break;
}
return rc;
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index a317f67ec0..29f5c7c279 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -1984,11 +1984,9 @@ int xenmem_add_to_physmap_one(
break;
case XENMAPSPACE_grant_table:
- rc = gnttab_map_frame(d, idx, gfn, &mfn);
+ rc = gnttab_map_frame_begin(d, idx, gfn, &mfn);
if ( rc )
return rc;
- /* Need to take care of the reference obtained in gnttab_map_frame().
*/
- page = mfn_to_page(mfn);
break;
case XENMAPSPACE_gmfn:
@@ -2070,19 +2068,28 @@ int xenmem_add_to_physmap_one(
put_gfn(d, gfn_x(gfn));
put_both:
- /*
- * In the XENMAPSPACE_gmfn case, we took a ref of the gfn at the top.
- * We also may need to transfer ownership of the page reference to our
- * caller.
- */
- if ( space == XENMAPSPACE_gmfn )
+ switch ( space )
{
+ case XENMAPSPACE_gmfn:
+ /*
+ * We took a ref of the gfn at the top. We also may need to transfer
+ * ownership of the page reference to our caller.
+ */
put_gfn(d, gmfn);
if ( !rc && extra.ppage )
{
*extra.ppage = page;
page = NULL;
}
+ break;
+
+ case XENMAPSPACE_grant_table:
+ /*
+ * We (gnttab_map_frame_begin()) acquired a lock and took a ref of the
+ * page underlying the MFN at the top.
+ */
+ gnttab_map_frame_end(d, mfn);
+ break;
}
if ( page )
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 6c77867f8c..e0f306f268 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -4242,7 +4242,8 @@ int gnttab_acquire_resource(
return rc;
}
-int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn, mfn_t
*mfn)
+int gnttab_map_frame_begin(
+ struct domain *d, unsigned long idx, gfn_t gfn, mfn_t *mfn)
{
int rc = 0;
struct grant_table *gt = d->grant_table;
@@ -4280,11 +4281,19 @@ int gnttab_map_frame(struct domain *d, unsigned long
idx, gfn_t gfn, mfn_t *mfn)
put_page(pg);
}
- grant_write_unlock(gt);
+ if ( rc )
+ grant_write_unlock(d->grant_table);
return rc;
}
+void gnttab_map_frame_end(struct domain *d, mfn_t mfn)
+{
+ put_page(mfn_to_page(mfn));
+
+ grant_write_unlock(d->grant_table);
+}
+
static void gnttab_usage_print(struct domain *rd)
{
int first = 1;
diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
index 50edfecfb6..a1db21e3fe 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -56,8 +56,13 @@ int gnttab_release_mappings(struct domain *d);
int mem_sharing_gref_to_gfn(struct grant_table *gt, grant_ref_t ref,
gfn_t *gfn, uint16_t *status);
-int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
- mfn_t *mfn);
+/*
+ * These need to be used as a pair, as the first (in the success case) returns
+ * with a lock and page reference held which the second needs to drop.
+ */
+int gnttab_map_frame_begin(struct domain *d, unsigned long idx, gfn_t gfn,
+ mfn_t *mfn);
+void gnttab_map_frame_end(struct domain *d, mfn_t mfn);
unsigned int gnttab_resource_max_frames(const struct domain *d, unsigned int
id);
@@ -96,12 +101,14 @@ static inline int mem_sharing_gref_to_gfn(struct
grant_table *gt,
return -EINVAL;
}
-static inline int gnttab_map_frame(struct domain *d, unsigned long idx,
- gfn_t gfn, mfn_t *mfn)
+static inline int gnttab_map_frame_begin(struct domain *d, unsigned long idx,
+ gfn_t gfn, mfn_t *mfn)
{
return -EINVAL;
}
+static inline void gnttab_map_frame_end(struct domain *d, mfn_t mfn) {}
+
static inline unsigned int gnttab_resource_max_frames(
const struct domain *d, unsigned int id)
{
--
generated by git-patchbot for /home/xen/git/xen.git#staging-4.20
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |