From: Markus Armbruster <armbru@redhat.com>
Date: Mon, 7 Jul 2008 11:37:19 -0400
Subject: [xen] PVFB probe & suspend fixes
Message-id: m3tzf1g0y8.fsf@crossbow.pond.sub.org
O-Subject: [PATCH RHEL-5.3 REPOST] Xen PVFB probe & suspend fixes
Bugzilla: 434800
RH-Acked-by: Chris Lalancette <clalance@redhat.com>
RH-Acked-by: Bill Burns <bburns@redhat.com>

This is four trivial fixes rolled into one patch.  I'm happy to split
it up if you'd rather have four separate patch files.

1. Virtual framebuffer probe can crash on an unlikely error path

   xenfb_probe() dereferences null info->fb_info when
   register_framebuffer() fails.  register_framebuffer() fails when
   FB_MAX framebuffers are already registered.  I doubt this can
   happen, because the device is compiled in and thus probes fairly
   early.

   Never reproduced.  Pvops PVFB in Linus's tree got this fix.

   Bug 434800: xenkbd can crash when probe fails

2. Fix virtual keyboard's test for empty ring

   The test compared against out_cons instead of in_cons.  out_cons is
   always zero with the current code.

   This can send a bogus event to the backend, which the backend
   ignores.  It can also delay processing of events until more arrive
   (very unlikely), or deadlock the virtual keyboard (very, very
   unlikely).

   Never reproduced.  Fixed in XenSource's kernel in
   http://xenbits.xensource.com/linux-2.6.18-xen.hg?rev/418
   Pvops PVFB in Linus's tree got the fix as well.

   Bug 451807: xenkbd's test for ring empty is wrong

3. Virtual keyboard can deadlock on resume

   xenkbd_resume() fails to reinitialize the shared page.  If the
   driver resumes with a full ring buffer, it deadlocks: the frontend
   won't drain the ring buffer until notified through the event
   channel, and the backend won't notify while the ring buffer is
   full.

   Fixed in XenSource's kernel in
   http://xenbits.xensource.com/linux-2.6.18-xen.hg?rev/415
   That fix clears just the defined members of the shared page.  My
   fix clears the whole shared page, both on probe and resume.  Makes
   more sense, and matches the framebuffer device's behavior.  Pvops
   PVFB in Linus's tree got my fix.

   Bug 434802: xenkbd can deadlock virtual keyboard on resume

4. Virtual keyboard probe crash on an unlikely error path

   When xenkbd_probe() fails, it calls xenkbd_remove() to clean up.

   If it fails before the input device successfully registered,
   info->dev is still null, and xenkbd_remove() must not call
   input_unregister_device() for it.

   If it fails before info->irq gets its real value, xenkbd_resume()
   calls unbind_from_irqhandler() for irq 0 when it should do nothing.

   Never reproduced.  Pvops PVFB in Linus's tree got this fix.

   Bug 434806: xenfb can crash when probe fails

Please ACK.

diff --git a/drivers/xen/fbfront/xenfb.c b/drivers/xen/fbfront/xenfb.c
index 9a9ebf8..f9aaf56 100644
--- a/drivers/xen/fbfront/xenfb.c
+++ b/drivers/xen/fbfront/xenfb.c
@@ -472,7 +472,7 @@ static int __devinit xenfb_probe(struct xenbus_device *dev,
 
 	ret = register_framebuffer(fb_info);
 	if (ret) {
-		fb_dealloc_cmap(&info->fb_info->cmap);
+		fb_dealloc_cmap(&fb_info->cmap);
 		framebuffer_release(fb_info);
 		xenbus_dev_fatal(dev, ret, "register_framebuffer");
 		goto error;
diff --git a/drivers/xen/fbfront/xenkbd.c b/drivers/xen/fbfront/xenkbd.c
index 7737732..ad04c54 100644
--- a/drivers/xen/fbfront/xenkbd.c
+++ b/drivers/xen/fbfront/xenkbd.c
@@ -52,7 +52,7 @@ static irqreturn_t input_handler(int rq, void *dev_id, struct pt_regs *regs)
 	__u32 cons, prod;
 
 	prod = page->in_prod;
-	if (prod == page->out_cons)
+	if (prod == page->in_cons)
 		return IRQ_HANDLED;
 	rmb();			/* ensure we see ring contents up to prod */
 	for (cons = page->in_cons; cons != prod; cons++) {
@@ -95,12 +95,11 @@ int __devinit xenkbd_probe(struct xenbus_device *dev,
 	}
 	dev->dev.driver_data = info;
 	info->xbdev = dev;
+	info->irq = -1;
 
-	info->page = (void *)__get_free_page(GFP_KERNEL);
+	info->page = (void *)__get_free_page(GFP_KERNEL || __GFP_ZERO);
 	if (!info->page)
 		goto error_nomem;
-	info->page->in_cons = info->page->in_prod = 0;
-	info->page->out_cons = info->page->out_prod = 0;
 
 	input_dev = input_allocate_device();
 	if (!input_dev)
@@ -148,6 +147,7 @@ static int xenkbd_resume(struct xenbus_device *dev)
 	struct xenkbd_info *info = dev->dev.driver_data;
 
 	xenkbd_disconnect_backend(info);
+	memset(info->page, 0, PAGE_SIZE);
 	return xenkbd_connect_backend(dev, info);
 }
 
@@ -156,7 +156,8 @@ static int xenkbd_remove(struct xenbus_device *dev)
 	struct xenkbd_info *info = dev->dev.driver_data;
 
 	xenkbd_disconnect_backend(info);
-	input_unregister_device(info->dev);
+	if (info->dev)
+		input_unregister_device(info->dev);
 	free_page((unsigned long)info->page);
 	kfree(info);
 	return 0;