[Xen-changelog] [xen-4.2-testing] tmem: detect arithmetic overflow in tmh_copy_{from, to}_client()

[Xen patchbot-4.2-testing <patchbot@xxxxxxx>]

# HG changeset patch
# User Jan Beulich <jbeulich@xxxxxxxx>
# Date 1348568101 -7200
# Node ID 61c35eaf1a88bb68816a18f2ae25dedd60f71ae0
# Parent  5bc3e774301c435c97b2d1a40193b19b8911bdfa
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@xxxxxxx>
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
Acked-by: Dan Magenheimer <dan.magenheimer@xxxxxxxxxx>
xen-unstable changeset: 25855:33b8c42a87ec
xen-unstable date: Tue Sep 11 12:17:59 UTC 2012
---


diff -r 5bc3e774301c -r 61c35eaf1a88 xen/common/tmem.c
--- a/xen/common/tmem.c Tue Sep 25 12:14:31 2012 +0200
+++ b/xen/common/tmem.c Tue Sep 25 12:15:01 2012 +0200
@@ -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);
+    else if ( pgp->size != 0 )
     {
-        rc = pcd_copy_to_client(cmfn, pgp);
-        if ( rc <= 0 )
-            goto bad_copy;
-    } 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;
 }
diff -r 5bc3e774301c -r 61c35eaf1a88 xen/common/tmem_xen.c
--- a/xen/common/tmem_xen.c     Tue Sep 25 12:14:31 2012 +0200
+++ b/xen/common/tmem_xen.c     Tue Sep 25 12:15:01 2012 +0200
@@ -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-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog