[PATCH] drm: do not expose vblank data before drm_vblank_init completes

[Marcin Slusarz]

On Thu, May 24, 2012 at 08:09:41PM +0200, Bruno Prémont wrote:
> I can easily trigger a crash in nouveau interrupt handler by unbinding
> and rebinding the GPU.
> 
> The command used:
>   echo $pci_device > nouveau/unbind && \
> 	sleep 5 && \
> 	echo $pci_device > nouveau/bind
> 
> 
> Kernel is 3.4.0 with modular drm/nouveau.
> GPU is NVidia nForce IGP (NV11)
> 
> 
> Unbinding seems to work fine, display switching back to VGA text mode.
> Rebinding the GPU slightly later causes the below trace:
> 
> (...)

It crashed because Nouveau failed to disable vblank interrupt on unbind
and this interrupt triggered while drm was initializing vblank data.

Nouveau side was fixed by "drm/nv04/disp: disable vblank interrupts when
disabling display" by Ben Skeggs (3.5 merge window), drm side can be fixed
by this:

---
From: Marcin Slusarz <marcin.slusarz at gmail.com>
Subject: [PATCH] drm: do not expose vblank data before drm_vblank_init completes

It fixes oops in drm_handle_vblank when vblank interrupt triggers before
drm_vblank_init completes. Driver side should not let this happen, but let's
be on the safe side and handle this case.

Reported-by: Bruno Prémont <bonbons at linux-vserver.org>
Tested-by: Bruno Prémont <bonbons at linux-vserver.org>
Signed-off-by: Marcin Slusarz <marcin.slusarz at gmail.com>
---
 drivers/gpu/drm/drm_irq.c |   21 +++++++++++++--------
 1 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index c869436..7dda18c 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -183,12 +183,8 @@ static void vblank_disable_fn(unsigned long arg)
 	}
 }
 
-void drm_vblank_cleanup(struct drm_device *dev)
+static void __drm_vblank_cleanup(struct drm_device *dev)
 {
-	/* Bail if the driver didn't call drm_vblank_init() */
-	if (dev->num_crtcs == 0)
-		return;
-
 	del_timer(&dev->vblank_disable_timer);
 
 	vblank_disable_fn((unsigned long)dev);
@@ -201,6 +197,15 @@ void drm_vblank_cleanup(struct drm_device *dev)
 	kfree(dev->last_vblank_wait);
 	kfree(dev->vblank_inmodeset);
 	kfree(dev->_vblank_time);
+}
+
+void drm_vblank_cleanup(struct drm_device *dev)
+{
+	/* Bail if the driver didn't call drm_vblank_init() */
+	if (dev->num_crtcs == 0)
+		return;
+
+	__drm_vblank_cleanup(dev);
 
 	dev->num_crtcs = 0;
 }
@@ -215,8 +220,6 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs)
 	spin_lock_init(&dev->vbl_lock);
 	spin_lock_init(&dev->vblank_time_lock);
 
-	dev->num_crtcs = num_crtcs;
-
 	dev->vbl_queue = kmalloc(sizeof(wait_queue_head_t) * num_crtcs,
 				 GFP_KERNEL);
 	if (!dev->vbl_queue)
@@ -268,10 +271,12 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs)
 	}
 
 	dev->vblank_disable_allowed = 0;
+	dev->num_crtcs = num_crtcs;
+
 	return 0;
 
 err:
-	drm_vblank_cleanup(dev);
+	__drm_vblank_cleanup(dev);
 	return ret;
 }
 EXPORT_SYMBOL(drm_vblank_init);
-- 
1.7.8.6