[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH 2/2] memory: XENMEM_add_to_physmap (almost) wrapping checks
Determining that behavior is correct (i.e. results in failure) for a passed in GFN equaling INVALID_GFN is non-trivial. Make this quite a bit more obvious by checking input in generic code - both for singular requests to not match the value and for range ones to not pass / wrap through it. For Arm similarly make more obvious that no wrapping of MFNs passed for XENMAPSPACE_dev_mmio and thus to map_dev_mmio_region() can occur: Drop the "nr" parameter of the function to avoid future callers appearing which might not themselves check for wrapping. Otherwise the respective ASSERT() in rangeset_contains_range() could trigger. Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> --- I find it odd that map_dev_mmio_region() returns success upon iomem_access_permitted() indicating failure - is this really intended? As per commit 102984bb1987 introducing it this also was added for ACPI only - any reason XENMAPSPACE_dev_mmio isn't restricted to CONFIG_ACPI builds? I'd be happy to take suggestions towards avoiding the need to #define _gfn() around the BUILD_BUG_ON() being added. I couldn't think of anything prettier. --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1479,7 +1479,7 @@ int xenmem_add_to_physmap_one( break; } case XENMAPSPACE_dev_mmio: - rc = map_dev_mmio_region(d, gfn, 1, _mfn(idx)); + rc = map_dev_mmio_region(d, gfn, _mfn(idx)); return rc; default: --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1360,19 +1360,18 @@ int unmap_mmio_regions(struct domain *d, int map_dev_mmio_region(struct domain *d, gfn_t gfn, - unsigned long nr, mfn_t mfn) { int res; - if ( !(nr && iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn) + nr - 1)) ) + if ( !iomem_access_permitted(d, mfn_x(mfn), mfn_x(mfn)) ) return 0; - res = p2m_insert_mapping(d, gfn, nr, mfn, p2m_mmio_direct_c); + res = p2m_insert_mapping(d, gfn, 1, mfn, p2m_mmio_direct_c); if ( res < 0 ) { - printk(XENLOG_G_ERR "Unable to map MFNs [%#"PRI_mfn" - %#"PRI_mfn" in Dom%d\n", - mfn_x(mfn), mfn_x(mfn) + nr - 1, d->domain_id); + printk(XENLOG_G_ERR "Unable to map MFN %#"PRI_mfn" in %pd\n", + mfn_x(mfn), d); return res; } --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -4132,7 +4132,10 @@ int gnttab_map_frame(struct domain *d, u bool status = false; if ( gfn_eq(gfn, INVALID_GFN) ) + { + ASSERT_UNREACHABLE(); return -EINVAL; + } grant_write_lock(gt); --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -831,6 +831,9 @@ int xenmem_add_to_physmap(struct domain return -EACCES; } + if ( gfn_eq(_gfn(xatp->gpfn), INVALID_GFN) ) + return -EINVAL; + if ( xatp->space == XENMAPSPACE_gmfn_foreign ) extra.foreign_domid = DOMID_INVALID; @@ -841,6 +844,15 @@ int xenmem_add_to_physmap(struct domain if ( xatp->size < start ) return -EILSEQ; + if ( xatp->gpfn + xatp->size < xatp->gpfn || + xatp->idx + xatp->size < xatp->idx ) + { +#define _gfn(x) (x) + BUILD_BUG_ON(INVALID_GFN + 1); +#undef _gfn + return -EOVERFLOW; + } + xatp->idx += start; xatp->gpfn += start; xatp->size -= start; @@ -961,6 +973,9 @@ static int xenmem_add_to_physmap_batch(s extent, 1)) ) return -EFAULT; + if ( gfn_eq(_gfn(gpfn), INVALID_GFN) ) + return -EINVAL; + rc = xenmem_add_to_physmap_one(d, xatpb->space, extra, idx, _gfn(gpfn)); --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -297,7 +297,6 @@ int unmap_regions_p2mt(struct domain *d, int map_dev_mmio_region(struct domain *d, gfn_t gfn, - unsigned long nr, mfn_t mfn); int guest_physmap_add_entry(struct domain *d,
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |