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.

Reported-by: Tim Deegan <tim@xen.org>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Dan Magenheimer <dan.magenheimer@oracle.com>
xen-unstable changeset: 25855:33b8c42a87ec
xen-unstable date: Tue Sep 11 12:17:59 UTC 2012

2 files changed, 19 insertions, 16 deletions

diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index ae02b379f7..b41f562ed6 100644
--- a/xen/common/tmem.c
+++ b/xen/common/tmem.c
@@ -1538,7 +1538,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) )
     {
@@ -1559,9 +1559,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:
@@ -1665,7 +1663,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) )
     {
@@ -1705,8 +1703,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:
@@ -1740,7 +1736,7 @@ static NOINLINE int do_tmem_get(pool_t *pool, OID *oidp, uint32_t index,
     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;
@@ -1764,20 +1760,20 @@ static NOINLINE int do_tmem_get(pool_t *pool, OID *oidp, uint32_t index,
     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) )
@@ -1814,7 +1810,6 @@ static NOINLINE int do_tmem_get(pool_t *pool, OID *oidp, uint32_t index,
     return 1;
 
 bad_copy:
-    /* this should only happen if the client passed a bad mfn */
     failed_copies++;
     return rc;
 }
diff --git a/xen/common/tmem_xen.c b/xen/common/tmem_xen.c
index bcd49f2afb..d549839cd6 100644
--- a/xen/common/tmem_xen.c
+++ b/xen/common/tmem_xen.c
@@ -148,6 +148,8 @@ EXPORT int tmh_copy_from_client(pfp_t *pfp,
     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);
@@ -178,6 +180,8 @@ EXPORT int tmh_copy_from_client(pfp_t *pfp,
                                          pfn_offset, len) )
             rc = -EFAULT;
     }
+    else if ( len )
+        rc = -EINVAL;
     if ( cli_va )
         cli_put_page(cli_va, cli_pfp, cli_mfn, 0);
     unmap_domain_page(tmem_va);
@@ -225,6 +229,8 @@ EXPORT int tmh_copy_to_client(tmem_cli_mfn_t cmfn, pfp_t *pfp,
     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) )
     {
@@ -244,6 +250,8 @@ EXPORT int tmh_copy_to_client(tmem_cli_mfn_t cmfn, pfp_t *pfp,
                                        tmem_va + tmem_offset, len) )
             rc = -EFAULT;
     }
+    else if ( len )
+        rc = -EINVAL;
     unmap_domain_page(tmem_va);
     if ( cli_va )
         cli_put_page(cli_va, cli_pfp, cli_mfn, 1);