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>

diff --git a/xen/common/tmem.c b/xen/common/tmem.c
index e6c3b38f890f01fceec6d9582400a9d799d2f97c..b1d0017e47b630cf9e72f970d6c905070512bb8e 100644 (file)
--- 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 *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;
@@ -1761,20 +1757,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) )
@@ -1811,7 +1807,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 f41db37e5e32ff2a702eded88e6f8b2693258535..78d2ede1bc7e8d66ab8e15fea9291463b6d6b19d 100644 (file)
--- a/xen/common/tmem_xen.c
+++ b/xen/common/tmem_xen.c
@@ -153,6 +153,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);
@@ -183,6 +185,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(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_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) )
     {
@@ -249,6 +255,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(cmfn, cli_va, cli_pfp, cli_mfn, 1);