tmem: detect arithmetic overflow in tmh_copy_{from,to}_client() This implies adjusting callers to deal with errors other than -EFAULT and removing some comments which would otherwise become stale. This is part of XSA-15 / CVE-2012-3497. Reported-by: Tim Deegan Signed-off-by: Jan Beulich Acked-by: Dan Magenheimer --- a/xen/common/tmem.c +++ b/xen/common/tmem.c @@ -1535,7 +1535,7 @@ copy_uncompressed: /* tmh_copy_from_client properly handles len==0 and offsets != 0 */ ret = tmh_copy_from_client(pgp->pfp, cmfn, tmem_offset, pfn_offset, len, tmh_cli_buf_null); - if ( ret == -EFAULT ) + if ( ret < 0 ) goto bad_copy; if ( tmh_dedup_enabled() && !is_persistent(pool) ) { @@ -1556,9 +1556,7 @@ done: return 1; bad_copy: - /* this should only happen if the client passed a bad mfn */ failed_copies++; - ret = -EFAULT; goto cleanup; failed_dup: @@ -1662,7 +1660,7 @@ copy_uncompressed: /* tmh_copy_from_client properly handles len==0 (TMEM_NEW_PAGE) */ ret = tmh_copy_from_client(pgp->pfp, cmfn, tmem_offset, pfn_offset, len, clibuf); - if ( ret == -EFAULT ) + if ( ret < 0 ) goto bad_copy; if ( tmh_dedup_enabled() && !is_persistent(pool) ) { @@ -1702,8 +1700,6 @@ insert_page: return 1; bad_copy: - /* this should only happen if the client passed a bad mfn */ - ret = -EFAULT; failed_copies++; delete_and_free: @@ -1737,7 +1733,7 @@ static NOINLINE int do_tmem_get(pool_t * pgp_t *pgp; client_t *client = pool->client; DECL_LOCAL_CYC_COUNTER(decompress); - int rc = -EFAULT; + int rc; if ( !_atomic_read(pool->pgp_count) ) return -EEMPTY; @@ -1761,20 +1757,20 @@ static NOINLINE int do_tmem_get(pool_t * ASSERT(pgp->size != -1); if ( tmh_dedup_enabled() && !is_persistent(pool) && pgp->firstbyte != NOT_SHAREABLE ) - { rc = pcd_copy_to_client(cmfn, pgp); - if ( rc <= 0 ) - goto bad_copy; - } else if ( pgp->size != 0 ) { + else if ( pgp->size != 0 ) + { START_CYC_COUNTER(decompress); rc = tmh_decompress_to_client(cmfn, pgp->cdata, pgp->size, clibuf); - if ( rc <= 0 ) - goto bad_copy; END_CYC_COUNTER(decompress); - } else if ( tmh_copy_to_client(cmfn, pgp->pfp, tmem_offset, - pfn_offset, len, clibuf) == -EFAULT) + } + else + rc = tmh_copy_to_client(cmfn, pgp->pfp, tmem_offset, + pfn_offset, len, clibuf); + if ( rc <= 0 ) goto bad_copy; + if ( is_ephemeral(pool) ) { if ( is_private(pool) ) @@ -1811,7 +1807,6 @@ static NOINLINE int do_tmem_get(pool_t * return 1; bad_copy: - /* this should only happen if the client passed a bad mfn */ failed_copies++; return rc; } --- a/xen/common/tmem_xen.c +++ b/xen/common/tmem_xen.c @@ -153,6 +153,8 @@ EXPORT int tmh_copy_from_client(pfp_t *p pfp_t *cli_pfp = NULL; int rc = 1; + if ( tmem_offset > PAGE_SIZE || pfn_offset > PAGE_SIZE || len > PAGE_SIZE ) + return -EINVAL; ASSERT(pfp != NULL); tmem_mfn = page_to_mfn(pfp); tmem_va = map_domain_page(tmem_mfn); @@ -183,6 +185,8 @@ EXPORT int tmh_copy_from_client(pfp_t *p pfn_offset, len) ) rc = -EFAULT; } + else if ( len ) + rc = -EINVAL; if ( cli_va ) cli_put_page(cmfn, cli_va, cli_pfp, cli_mfn, 0); unmap_domain_page(tmem_va); @@ -230,6 +234,8 @@ EXPORT int tmh_copy_to_client(tmem_cli_m pfp_t *cli_pfp = NULL; int rc = 1; + if ( tmem_offset > PAGE_SIZE || pfn_offset > PAGE_SIZE || len > PAGE_SIZE ) + return -EINVAL; ASSERT(pfp != NULL); if ( guest_handle_is_null(clibuf) ) { @@ -249,6 +255,8 @@ EXPORT int tmh_copy_to_client(tmem_cli_m tmem_va + tmem_offset, len) ) rc = -EFAULT; } + else if ( len ) + rc = -EINVAL; unmap_domain_page(tmem_va); if ( cli_va ) cli_put_page(cmfn, cli_va, cli_pfp, cli_mfn, 1);