[Xen-changelog] [xen-3.2-testing] ioemu: Fix PVFB backend to validate frontend's frame buffer

["Xen patchbot-3.2-testing" <patchbot-3.2-testing@xxxxxxxxxxxxxxxxxxx>]

# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1210689273 -3600
# Node ID f70475e8396dc4bc0304d5ff697f18e2b35926f4
# Parent  01f12d39071efa0b5e0d28c06584a42f19c49437
ioemu: Fix PVFB backend to validate frontend's frame buffer
description

A buggy or malicious frontend can describe its shared framebuffer to
the backend in a way that makes the backend map an arbitrary amount of
guest memory, malloc an arbitrarily large internal buffer, copy
arbitrary memory to that buffer, even beyond its end.  A domU running
a malicious frontend can abuse the former two for denial of service
attacks against dom0.  It can abuse the third to write arbitrary
backend memory.  It can abuse all three to terminate or crash the
backend.  Arbitrary code execution looks quite feasible.

From: Markus Armbruster <armbru@xxxxxxxxxx>
Signed-off-by: Keir Fraser <keir.fraser@xxxxxxxxxx>
xen-unstable changeset:   17630:53195719f7621110dab7a97a2bca292b73baa715
xen-unstable date:        Tue May 13 15:08:17 2008 +0100
---
 tools/ioemu/hw/xenfb.c |   64 ++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 53 insertions(+), 11 deletions(-)

diff -r 01f12d39071e -r f70475e8396d tools/ioemu/hw/xenfb.c
--- a/tools/ioemu/hw/xenfb.c    Tue May 13 15:23:51 2008 +0100
+++ b/tools/ioemu/hw/xenfb.c    Tue May 13 15:34:33 2008 +0100
@@ -22,8 +22,6 @@
 #ifndef BTN_LEFT
 #define BTN_LEFT 0x110 /* from <linux/input.h> */
 #endif
-
-// FIXME defend against malicious frontend?
 
 struct xenfb;
 
@@ -476,6 +474,50 @@ void xenfb_shutdown(struct xenfb *xenfb)
        free(xenfb);
 }
 
+static int xenfb_configure_fb(struct xenfb *xenfb,
+                             int width, int height, int depth,
+                             size_t fb_len, int row_stride)
+{
+       size_t mfn_sz = sizeof(*((struct xenfb_page *)0)->pd);
+       size_t pd_len = sizeof(((struct xenfb_page *)0)->pd) / mfn_sz;
+       size_t fb_pages = pd_len * XC_PAGE_SIZE / mfn_sz;
+       size_t fb_len_max = fb_pages * XC_PAGE_SIZE;
+       int max_width, max_height;
+
+       if (depth != 8 && depth != 16 && depth != 24 && depth != 32) {
+               fprintf(stderr,
+                       "FB: can't handle frontend fb depth %d\n",
+                       depth);
+               return -1;
+       }
+       if (row_stride < 0 || row_stride > fb_len) {
+               fprintf(stderr,
+                       "FB: invalid frontend stride %d\n", row_stride);
+               return -1;
+       }
+       max_width = row_stride / (depth / 8);
+       if (width < 0 || width > max_width) {
+               fprintf(stderr,
+                       "FB: invalid frontend width %d limited to %d\n",
+                       width, max_width);
+               width = max_width;
+       }
+       max_height = fb_len / row_stride;
+       if (height < 0 || height > max_height) {
+               fprintf(stderr,
+                       "FB: invalid frontend height %d limited to %d\n",
+                       height, max_height);
+               height = max_height;
+       }
+       xenfb->fb_len = fb_len;
+       xenfb->row_stride = row_stride;
+       xenfb->depth = depth;
+       xenfb->width = width;
+       xenfb->height = height;
+       fprintf(stderr, "Framebuffer %dx%dx%d stride %d\n",
+               width, height, depth, row_stride);
+       return 0;
+}
 
 static void xenfb_on_fb_event(struct xenfb *xenfb)
 {
@@ -506,7 +548,6 @@ static void xenfb_on_fb_event(struct xen
                            || h != event->update.height) {
                                fprintf(stderr, "%s bogus update clipped\n",
                                        xenfb->fb.nodename);
-                               break;
                        }
                        xenfb_guest_copy(xenfb, x, y, w, h);
                        break;
@@ -686,14 +727,15 @@ static int xenfb_read_frontend_fb_config
                 xenfb->protocol[0] = '\0';
         xenfb_xs_printf(xenfb->xsh, xenfb->fb.nodename, "request-update", "1");
 
-        /* TODO check for permitted ranges */
-        fb_page = xenfb->fb.page;
-        xenfb->depth = fb_page->depth;
-        xenfb->width = fb_page->width;
-        xenfb->height = fb_page->height;
-        /* TODO check for consistency with the above */
-        xenfb->fb_len = fb_page->mem_length;
-        xenfb->row_stride = fb_page->line_length;
+       fb_page = xenfb->fb.page;
+       if (xenfb_configure_fb(xenfb,
+                              fb_page->width, fb_page->height, fb_page->depth,
+                              fb_page->mem_length, fb_page->line_length)
+           < 0) {
+               errno = EINVAL;
+               return -1;
+       }
+
         fprintf(stderr, "Framebuffer depth %d width %d height %d line %d\n",
                 fb_page->depth, fb_page->width, fb_page->height, 
fb_page->line_length);
         if (xenfb_map_fb(xenfb, xenfb->fb.otherend_id) < 0)

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