Skip to contentSkip to conversation

Bogus screen updates

PATCH New Markus Armbruster 1 participant 3 messages Nov 13, 2007 Xen hypervisor interface

lore
Share & tools
lore

Markus Armbruster

author
#0 lore raw

cover Bogus screen updates

The PVFB frontend can get confused and send a bogus screen update to the back end when the screen is clean.

Two parts: fix the frontend not to do that, and fix the backend not to choke on it.

author
#1 lore raw

PATCH 1/2 PVFB frontend can send bogus screen updates

The PVFB frontend can get confused and send a screen update to the backend when the screen is actually clean. Such an update asks for the impossible rectangle (x1, x2, y1, y2) = (INT_MAX, 0, INT_MAX, 0). Fix by setting the dirty flag in the obvious place: when the dirty rectangle is grown.

Signed-off-by Markus Armbruster <[email protected]>
drivers/xen/fbfront/xenfb.c +6 -1
diff -r cf8b6cafa2f0 drivers/xen/fbfront/xenfb.c
@@ -201,6 +201,11 @@ static void xenfb_update_screen(struct x
 
 	mutex_unlock(&info->mm_lock);
 
+	if (x2 < x1 || y2 < y1) {
+		printk("xenfb_update_screen bogus rect %d %d %d %d\n",
+		       x1, x2, y1, y2);
+		WARN_ON(1);
+	}
 	xenfb_do_update(info, x1, y1, x2 - x1, y2 - y1);
 }
 

@@ -252,7 +257,6 @@ static void xenfb_timer(unsigned long da
 static void xenfb_timer(unsigned long data)
 {
 	struct xenfb_info *info = (struct xenfb_info *)data;
-	info->dirty = 1;
 	wake_up(&info->wq);
 }
 

@@ -272,6 +276,7 @@ static void __xenfb_refresh(struct xenfb
 		info->x1 = x1;
 	if (info->x2 < x2)
 		info->x2 = x2;
+	info->dirty = 1;
 
 	if (timer_pending(&info->refresh))
 		return;
author
#2 lore raw

PATCH 2/2 PVFB SDL backend chokes on bogus screen updates

In reply to Markus Armbruster

Bogus screen update requests from buggy or malicous frontend make SDL crash. The VNC backend silently ignores them. Catch and log them.

Signed-off-by Markus Armbruster <[email protected]>
tools/ioemu/hw/xenfb.c +18 -3
diff -r 837f83225153 tools/ioemu/hw/xenfb.c
@@ -488,12 +488,27 @@ static void xenfb_on_fb_event(struct xen
 	rmb();			/* ensure we see ring contents up to prod */
 	for (cons = page->out_cons; cons != prod; cons++) {
 		union xenfb_out_event *event = &XENFB_OUT_RING_REF(page, cons);
+		int x, y, w, h;
 
 		switch (event->type) {
 		case XENFB_TYPE_UPDATE:
-			xenfb_guest_copy(xenfb,
-					 event->update.x, event->update.y,
-					 event->update.width, event->update.height);
+			x = MAX(event->update.x, 0);
+			y = MAX(event->update.y, 0);
+			w = MIN(event->update.width, xenfb->width - x);
+			h = MIN(event->update.height, xenfb->height - y);
+			if (w < 0 || h < 0) {
+				fprintf(stderr, "%s bogus update ignored\n",
+					xenfb->fb.nodename);
+				break;
+			}
+			if (x != event->update.x || y != event->update.y
+			    || w != event->update.width
+			    || h != event->update.height) {
+				fprintf(stderr, "%s bogus update clipped\n",
+					xenfb->fb.nodename);
+				break;
+			}
+			xenfb_guest_copy(xenfb, x, y, w, h);
 			break;
 		}
 	}