Jan Beulich
2012-Sep-05 12:37 UTC
[PATCH 06/11] 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 <tim@xen.org>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Dan Magenheimer <dan.magenheimer@oracle.com>
--- 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);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel