All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] [xen-4.0-testing] tmem: disallow bad gmfns from PV domains
@ 2010-09-21 19:07 Dan Magenheimer
  0 siblings, 0 replies; only message in thread
From: Dan Magenheimer @ 2010-09-21 19:07 UTC (permalink / raw)
  To: Xen-devel; +Cc: Keir Fraser

[-- Attachment #1: Type: text/plain, Size: 7836 bytes --]

(Same patch, rebased for 4.0.x)

Tmem: disallow bad gmfns from PV domains

Mfns for PV domains were not properly checked, potentially
allowing a buggy or malicious PV guest to crash Xen.  Also,
use get_page/put_page to claim a reference to the pages
so they can't disappear out from under tmem's feet.

Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>

diff -r 89605b79f565 xen/common/tmem_xen.c
--- a/xen/common/tmem_xen.c	Mon Sep 13 17:51:50 2010 +0100
+++ b/xen/common/tmem_xen.c	Tue Sep 21 13:04:20 2010 -0600
@@ -87,49 +87,88 @@
 }
 
 #ifdef __ia64__
-static inline void *cli_mfn_to_va(tmem_cli_mfn_t cmfn, unsigned long *pcli_mfn)
+static inline void *cli_get_page(tmem_cli_mfn_t cmfn, unsigned long *pcli_mfn,
+                                 pfp_t **pcli_pfp, bool_t cli_write)
 {
     ASSERT(0);
     return NULL;
 }
-#define paging_mark_dirty(_x,_y) do {} while(0)
+
+static inline void cli_put_page(void *cli_va, struct page_info *cli_pfp,
+                                bool_t mark_dirty)
+{
+    ASSERT(0);
+}
 #else
-static inline void *cli_mfn_to_va(tmem_cli_mfn_t cmfn, unsigned long *pcli_mfn)
+static inline void *cli_get_page(tmem_cli_mfn_t cmfn, unsigned long *pcli_mfn,
+                                 pfp_t **pcli_pfp, bool_t cli_write)
 {
     unsigned long cli_mfn;
     p2m_type_t t;
+    struct page_info *page;
+    int ret;
 
     cli_mfn = mfn_x(gfn_to_mfn(current->domain, cmfn, &t));
-    if (t != p2m_ram_rw || cli_mfn == INVALID_MFN)
+    if ( t != p2m_ram_rw || !mfn_valid(cli_mfn) )
+            return NULL;
+    page = mfn_to_page(cli_mfn);
+    if ( cli_write )
+        ret = get_page_and_type(page, current->domain, PGT_writable_page);
+    else
+        ret = get_page(page, current->domain);
+    if ( !ret )
         return NULL;
-    if (pcli_mfn != NULL)
-        *pcli_mfn = cli_mfn;
+    *pcli_mfn = cli_mfn;
+    *pcli_pfp = (pfp_t *)page;
     return map_domain_page(cli_mfn);
 }
+
+static inline void cli_put_page(void *cli_va, pfp_t *cli_pfp,
+                                unsigned long cli_mfn, bool_t mark_dirty)
+{
+    if ( mark_dirty )
+    {
+        paging_mark_dirty(current->domain,cli_mfn);
+        put_page_and_type((struct page_info *)cli_pfp);
+    }
+    else
+        put_page((struct page_info *)cli_pfp);
+    unmap_domain_page(cli_va);
+}
 #endif
 
 EXPORT int tmh_copy_from_client(pfp_t *pfp,
     tmem_cli_mfn_t cmfn, pagesize_t tmem_offset,
     pagesize_t pfn_offset, pagesize_t len, void *cli_va)
 {
-    unsigned long tmem_mfn;
+    unsigned long tmem_mfn, cli_mfn = 0;
     void *tmem_va;
+    pfp_t *cli_pfp;
+    bool_t tmemc = cli_va != NULL; /* if true, cli_va is control-op buffer */
 
     ASSERT(pfp != NULL);
-    if ( tmem_offset || pfn_offset || len )
-        if ( (cli_va == NULL) && ((cli_va = cli_mfn_to_va(cmfn,NULL)) == NULL) )
-            return -EFAULT;
     tmem_mfn = page_to_mfn(pfp);
     tmem_va = map_domain_page(tmem_mfn);
+    if ( tmem_offset == 0 && pfn_offset == 0 && len == 0 )
+    {
+        memset(tmem_va, 0, PAGE_SIZE);
+        unmap_domain_page(tmem_va);
+        return 1;
+    }
+    if ( !tmemc )
+    {
+        cli_va = cli_get_page(cmfn, &cli_mfn, &cli_pfp, 0);
+        if ( cli_va == NULL )
+            return -EFAULT;
+    }
     mb();
-    if (!len && !tmem_offset && !pfn_offset)
-        memset(tmem_va, 0, PAGE_SIZE);
-    else if (len == PAGE_SIZE && !tmem_offset && !pfn_offset)
+    if (len == PAGE_SIZE && !tmem_offset && !pfn_offset)
         tmh_copy_page(tmem_va, cli_va);
     else if ( (tmem_offset+len <= PAGE_SIZE) &&
-                (pfn_offset+len <= PAGE_SIZE) ) 
+              (pfn_offset+len <= PAGE_SIZE) )
         memcpy((char *)tmem_va+tmem_offset,(char *)cli_va+pfn_offset,len);
-    unmap_domain_page(cli_va);
+    if ( !tmemc )
+        cli_put_page(cli_va, cli_pfp, cli_mfn, 0);
     unmap_domain_page(tmem_va);
     return 1;
 }
@@ -140,15 +179,24 @@
     int ret = 0;
     unsigned char *dmem = this_cpu(dstmem);
     unsigned char *wmem = this_cpu(workmem);
+    pfp_t *cli_pfp;
+    unsigned long cli_mfn = 0;
+    bool_t tmemc = cli_va != NULL; /* if true, cli_va is control-op buffer */
 
-    if ( (cli_va == NULL) && (cli_va = cli_mfn_to_va(cmfn,NULL)) == NULL)
-        return -EFAULT;
     if ( dmem == NULL || wmem == NULL )
         return 0;  /* no buffer, so can't compress */
+    if ( !tmemc )
+    {
+        cli_va = cli_get_page(cmfn, &cli_mfn, &cli_pfp, 0);
+        if ( cli_va == NULL )
+            return -EFAULT;
+    }
     mb();
     ret = lzo1x_1_compress(cli_va, PAGE_SIZE, dmem, out_len, wmem);
     ASSERT(ret == LZO_E_OK);
     *out_va = dmem;
+    if ( !tmemc )
+        cli_put_page(cli_va, cli_pfp, cli_mfn, 0);
     unmap_domain_page(cli_va);
     return 1;
 }
@@ -157,14 +205,17 @@
     pagesize_t tmem_offset, pagesize_t pfn_offset, pagesize_t len, void *cli_va)
 {
     unsigned long tmem_mfn, cli_mfn = 0;
-    int mark_dirty = 1;
     void *tmem_va;
+    pfp_t *cli_pfp;
+    bool_t tmemc = cli_va != NULL; /* if true, cli_va is control-op buffer */
 
     ASSERT(pfp != NULL);
-    if ( cli_va != NULL )
-        mark_dirty = 0;
-    else if ( (cli_va = cli_mfn_to_va(cmfn,&cli_mfn)) == NULL)
-        return -EFAULT;
+    if ( !tmemc )
+    {
+        cli_va = cli_get_page(cmfn, &cli_mfn, &cli_pfp, 1);
+        if ( cli_va == NULL )
+            return -EFAULT;
+    }
     tmem_mfn = page_to_mfn(pfp);
     tmem_va = map_domain_page(tmem_mfn);
     if (len == PAGE_SIZE && !tmem_offset && !pfn_offset)
@@ -172,11 +223,8 @@
     else if ( (tmem_offset+len <= PAGE_SIZE) && (pfn_offset+len <= PAGE_SIZE) )
         memcpy((char *)cli_va+pfn_offset,(char *)tmem_va+tmem_offset,len);
     unmap_domain_page(tmem_va);
-    if ( mark_dirty )
-    {
-        unmap_domain_page(cli_va);
-        paging_mark_dirty(current->domain,cli_mfn);
-    }
+    if ( !tmemc )
+        cli_put_page(cli_va, cli_pfp, cli_mfn, 1);
     mb();
     return 1;
 }
@@ -185,22 +233,22 @@
                                     size_t size, void *cli_va)
 {
     unsigned long cli_mfn = 0;
-    int mark_dirty = 1;
+    pfp_t *cli_pfp;
     size_t out_len = PAGE_SIZE;
+    bool_t tmemc = cli_va != NULL; /* if true, cli_va is control-op buffer */
     int ret;
 
-    if ( cli_va != NULL )
-        mark_dirty = 0;
-    else if ( (cli_va = cli_mfn_to_va(cmfn,&cli_mfn)) == NULL)
-        return -EFAULT;
+    if ( !tmemc )
+    {
+        cli_va = cli_get_page(cmfn, &cli_mfn, &cli_pfp, 1);
+        if ( cli_va == NULL )
+            return -EFAULT;
+    }
     ret = lzo1x_decompress_safe(tmem_va, size, cli_va, &out_len);
     ASSERT(ret == LZO_E_OK);
     ASSERT(out_len == PAGE_SIZE);
-    if ( mark_dirty )
-    {
-        unmap_domain_page(cli_va);
-        paging_mark_dirty(current->domain,cli_mfn);
-    }
+    if ( !tmemc )
+        cli_put_page(cli_va, cli_pfp, cli_mfn, 1);
     mb();
     return 1;
 }
@@ -210,18 +258,19 @@
 {
     void *cli_va;
     unsigned long cli_mfn;
+    pfp_t *cli_pfp;
 
     ASSERT(!(len & (sizeof(uint64_t)-1)));
     ASSERT(len <= PAGE_SIZE);
     ASSERT(len > 0 || tmem_va == NULL);
-    if ( (cli_va = cli_mfn_to_va(cmfn,&cli_mfn)) == NULL)
+    cli_va = cli_get_page(cmfn, &cli_mfn, &cli_pfp, 1);
+    if ( cli_va == NULL )
         return -EFAULT;
     if ( len > 0 )
         memcpy((char *)cli_va,(char *)tmem_va,len);
     if ( len < PAGE_SIZE )
         memset((char *)cli_va+len,0,PAGE_SIZE-len);
-    unmap_domain_page(cli_va);
-    paging_mark_dirty(current->domain,cli_mfn);
+    cli_put_page(cli_va, cli_pfp, cli_mfn, 1);
     mb();
     return 1;
 }

[-- Attachment #2: tmem-badgmfn-4.0.patch --]
[-- Type: application/octet-stream, Size: 7234 bytes --]

diff -r 89605b79f565 xen/common/tmem_xen.c
--- a/xen/common/tmem_xen.c	Mon Sep 13 17:51:50 2010 +0100
+++ b/xen/common/tmem_xen.c	Tue Sep 21 13:04:20 2010 -0600
@@ -87,49 +87,88 @@
 }
 
 #ifdef __ia64__
-static inline void *cli_mfn_to_va(tmem_cli_mfn_t cmfn, unsigned long *pcli_mfn)
+static inline void *cli_get_page(tmem_cli_mfn_t cmfn, unsigned long *pcli_mfn,
+                                 pfp_t **pcli_pfp, bool_t cli_write)
 {
     ASSERT(0);
     return NULL;
 }
-#define paging_mark_dirty(_x,_y) do {} while(0)
+
+static inline void cli_put_page(void *cli_va, struct page_info *cli_pfp,
+                                bool_t mark_dirty)
+{
+    ASSERT(0);
+}
 #else
-static inline void *cli_mfn_to_va(tmem_cli_mfn_t cmfn, unsigned long *pcli_mfn)
+static inline void *cli_get_page(tmem_cli_mfn_t cmfn, unsigned long *pcli_mfn,
+                                 pfp_t **pcli_pfp, bool_t cli_write)
 {
     unsigned long cli_mfn;
     p2m_type_t t;
+    struct page_info *page;
+    int ret;
 
     cli_mfn = mfn_x(gfn_to_mfn(current->domain, cmfn, &t));
-    if (t != p2m_ram_rw || cli_mfn == INVALID_MFN)
+    if ( t != p2m_ram_rw || !mfn_valid(cli_mfn) )
+            return NULL;
+    page = mfn_to_page(cli_mfn);
+    if ( cli_write )
+        ret = get_page_and_type(page, current->domain, PGT_writable_page);
+    else
+        ret = get_page(page, current->domain);
+    if ( !ret )
         return NULL;
-    if (pcli_mfn != NULL)
-        *pcli_mfn = cli_mfn;
+    *pcli_mfn = cli_mfn;
+    *pcli_pfp = (pfp_t *)page;
     return map_domain_page(cli_mfn);
 }
+
+static inline void cli_put_page(void *cli_va, pfp_t *cli_pfp,
+                                unsigned long cli_mfn, bool_t mark_dirty)
+{
+    if ( mark_dirty )
+    {
+        paging_mark_dirty(current->domain,cli_mfn);
+        put_page_and_type((struct page_info *)cli_pfp);
+    }
+    else
+        put_page((struct page_info *)cli_pfp);
+    unmap_domain_page(cli_va);
+}
 #endif
 
 EXPORT int tmh_copy_from_client(pfp_t *pfp,
     tmem_cli_mfn_t cmfn, pagesize_t tmem_offset,
     pagesize_t pfn_offset, pagesize_t len, void *cli_va)
 {
-    unsigned long tmem_mfn;
+    unsigned long tmem_mfn, cli_mfn = 0;
     void *tmem_va;
+    pfp_t *cli_pfp;
+    bool_t tmemc = cli_va != NULL; /* if true, cli_va is control-op buffer */
 
     ASSERT(pfp != NULL);
-    if ( tmem_offset || pfn_offset || len )
-        if ( (cli_va == NULL) && ((cli_va = cli_mfn_to_va(cmfn,NULL)) == NULL) )
-            return -EFAULT;
     tmem_mfn = page_to_mfn(pfp);
     tmem_va = map_domain_page(tmem_mfn);
+    if ( tmem_offset == 0 && pfn_offset == 0 && len == 0 )
+    {
+        memset(tmem_va, 0, PAGE_SIZE);
+        unmap_domain_page(tmem_va);
+        return 1;
+    }
+    if ( !tmemc )
+    {
+        cli_va = cli_get_page(cmfn, &cli_mfn, &cli_pfp, 0);
+        if ( cli_va == NULL )
+            return -EFAULT;
+    }
     mb();
-    if (!len && !tmem_offset && !pfn_offset)
-        memset(tmem_va, 0, PAGE_SIZE);
-    else if (len == PAGE_SIZE && !tmem_offset && !pfn_offset)
+    if (len == PAGE_SIZE && !tmem_offset && !pfn_offset)
         tmh_copy_page(tmem_va, cli_va);
     else if ( (tmem_offset+len <= PAGE_SIZE) &&
-                (pfn_offset+len <= PAGE_SIZE) ) 
+              (pfn_offset+len <= PAGE_SIZE) )
         memcpy((char *)tmem_va+tmem_offset,(char *)cli_va+pfn_offset,len);
-    unmap_domain_page(cli_va);
+    if ( !tmemc )
+        cli_put_page(cli_va, cli_pfp, cli_mfn, 0);
     unmap_domain_page(tmem_va);
     return 1;
 }
@@ -140,15 +179,24 @@
     int ret = 0;
     unsigned char *dmem = this_cpu(dstmem);
     unsigned char *wmem = this_cpu(workmem);
+    pfp_t *cli_pfp;
+    unsigned long cli_mfn = 0;
+    bool_t tmemc = cli_va != NULL; /* if true, cli_va is control-op buffer */
 
-    if ( (cli_va == NULL) && (cli_va = cli_mfn_to_va(cmfn,NULL)) == NULL)
-        return -EFAULT;
     if ( dmem == NULL || wmem == NULL )
         return 0;  /* no buffer, so can't compress */
+    if ( !tmemc )
+    {
+        cli_va = cli_get_page(cmfn, &cli_mfn, &cli_pfp, 0);
+        if ( cli_va == NULL )
+            return -EFAULT;
+    }
     mb();
     ret = lzo1x_1_compress(cli_va, PAGE_SIZE, dmem, out_len, wmem);
     ASSERT(ret == LZO_E_OK);
     *out_va = dmem;
+    if ( !tmemc )
+        cli_put_page(cli_va, cli_pfp, cli_mfn, 0);
     unmap_domain_page(cli_va);
     return 1;
 }
@@ -157,14 +205,17 @@
     pagesize_t tmem_offset, pagesize_t pfn_offset, pagesize_t len, void *cli_va)
 {
     unsigned long tmem_mfn, cli_mfn = 0;
-    int mark_dirty = 1;
     void *tmem_va;
+    pfp_t *cli_pfp;
+    bool_t tmemc = cli_va != NULL; /* if true, cli_va is control-op buffer */
 
     ASSERT(pfp != NULL);
-    if ( cli_va != NULL )
-        mark_dirty = 0;
-    else if ( (cli_va = cli_mfn_to_va(cmfn,&cli_mfn)) == NULL)
-        return -EFAULT;
+    if ( !tmemc )
+    {
+        cli_va = cli_get_page(cmfn, &cli_mfn, &cli_pfp, 1);
+        if ( cli_va == NULL )
+            return -EFAULT;
+    }
     tmem_mfn = page_to_mfn(pfp);
     tmem_va = map_domain_page(tmem_mfn);
     if (len == PAGE_SIZE && !tmem_offset && !pfn_offset)
@@ -172,11 +223,8 @@
     else if ( (tmem_offset+len <= PAGE_SIZE) && (pfn_offset+len <= PAGE_SIZE) )
         memcpy((char *)cli_va+pfn_offset,(char *)tmem_va+tmem_offset,len);
     unmap_domain_page(tmem_va);
-    if ( mark_dirty )
-    {
-        unmap_domain_page(cli_va);
-        paging_mark_dirty(current->domain,cli_mfn);
-    }
+    if ( !tmemc )
+        cli_put_page(cli_va, cli_pfp, cli_mfn, 1);
     mb();
     return 1;
 }
@@ -185,22 +233,22 @@
                                     size_t size, void *cli_va)
 {
     unsigned long cli_mfn = 0;
-    int mark_dirty = 1;
+    pfp_t *cli_pfp;
     size_t out_len = PAGE_SIZE;
+    bool_t tmemc = cli_va != NULL; /* if true, cli_va is control-op buffer */
     int ret;
 
-    if ( cli_va != NULL )
-        mark_dirty = 0;
-    else if ( (cli_va = cli_mfn_to_va(cmfn,&cli_mfn)) == NULL)
-        return -EFAULT;
+    if ( !tmemc )
+    {
+        cli_va = cli_get_page(cmfn, &cli_mfn, &cli_pfp, 1);
+        if ( cli_va == NULL )
+            return -EFAULT;
+    }
     ret = lzo1x_decompress_safe(tmem_va, size, cli_va, &out_len);
     ASSERT(ret == LZO_E_OK);
     ASSERT(out_len == PAGE_SIZE);
-    if ( mark_dirty )
-    {
-        unmap_domain_page(cli_va);
-        paging_mark_dirty(current->domain,cli_mfn);
-    }
+    if ( !tmemc )
+        cli_put_page(cli_va, cli_pfp, cli_mfn, 1);
     mb();
     return 1;
 }
@@ -210,18 +258,19 @@
 {
     void *cli_va;
     unsigned long cli_mfn;
+    pfp_t *cli_pfp;
 
     ASSERT(!(len & (sizeof(uint64_t)-1)));
     ASSERT(len <= PAGE_SIZE);
     ASSERT(len > 0 || tmem_va == NULL);
-    if ( (cli_va = cli_mfn_to_va(cmfn,&cli_mfn)) == NULL)
+    cli_va = cli_get_page(cmfn, &cli_mfn, &cli_pfp, 1);
+    if ( cli_va == NULL )
         return -EFAULT;
     if ( len > 0 )
         memcpy((char *)cli_va,(char *)tmem_va,len);
     if ( len < PAGE_SIZE )
         memset((char *)cli_va+len,0,PAGE_SIZE-len);
-    unmap_domain_page(cli_va);
-    paging_mark_dirty(current->domain,cli_mfn);
+    cli_put_page(cli_va, cli_pfp, cli_mfn, 1);
     mb();
     return 1;
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2010-09-21 19:07 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-21 19:07 [PATCH] [xen-4.0-testing] tmem: disallow bad gmfns from PV domains Dan Magenheimer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.