From: Márton Németh nm127@freemail.hu
The field obejct_idr of struct drm_via_private was introduced with the commit http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=... . In that patch idr_init(&dev->object_name_idr) was called instead of idr_init(&dev_priv->object_idr) by mistake, leaving the dev_priv->object_idr uninitialized. To be more exact, the object_idr buffer is filled with zeros because of kzalloc(), but the dev_priv->object_idr.lock spinlock can cause system freeze at lib/idr.c:move_to_free_list() when spin_lock_irqsave() is called on this spinlock.
The patch was tested on Clevo D4J, model D410J laptop, on the following hardware, without AGP kernel module loaded:
# lspci -s 01:00.0 -n 01:00.0 0300: 1106:3108 (rev 01) # lspci -s 01:00.0 -v 01:00.0 VGA compatible controller: VIA Technologies, Inc. K8M800/K8N800/K8N800A [S3 UniChrome Pro] (rev 01) (prog-if 00 [VGA controller]) Subsystem: CLEVO/KAPOK Computer Device 4702 Flags: bus master, 66MHz, medium devsel, latency 64, IRQ 16 Memory at f0000000 (32-bit, prefetchable) [size=64M] Memory at d1000000 (32-bit, non-prefetchable) [size=16M] Expansion ROM at <unassigned> [disabled] Capabilities: [60] Power Management version 2 Capabilities: [70] AGP version 3.0
Signed-off-by: Márton Németh nm127@freemail.hu --- diff --git a/drivers/gpu/drm/via/via_map.c b/drivers/gpu/drm/via/via_map.c index 1f18225..c126182 100644 --- a/drivers/gpu/drm/via/via_map.c +++ b/drivers/gpu/drm/via/via_map.c @@ -100,12 +100,11 @@ int via_driver_load(struct drm_device *dev, unsigned long chipset) if (dev_priv == NULL) return -ENOMEM;
+ idr_init(&dev_priv->object_idr); dev->dev_private = (void *)dev_priv;
dev_priv->chipset = chipset;
- idr_init(&dev->object_name_idr); - pci_set_master(dev->pdev);
ret = drm_vblank_init(dev, 1);
On Sun, Jun 10, 2012 at 11:39:55PM +0200, Németh Márton wrote:
From: Márton Németh nm127@freemail.hu
The field obejct_idr of struct drm_via_private was introduced with the commit http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=... . In that patch idr_init(&dev->object_name_idr) was called instead of idr_init(&dev_priv->object_idr) by mistake, leaving the dev_priv->object_idr uninitialized. To be more exact, the object_idr buffer is filled with zeros because of kzalloc(), but the dev_priv->object_idr.lock spinlock can cause system freeze at lib/idr.c:move_to_free_list() when spin_lock_irqsave() is called on this spinlock.
The patch was tested on Clevo D4J, model D410J laptop, on the following hardware, without AGP kernel module loaded:
# lspci -s 01:00.0 -n 01:00.0 0300: 1106:3108 (rev 01) # lspci -s 01:00.0 -v 01:00.0 VGA compatible controller: VIA Technologies, Inc. K8M800/K8N800/K8N800A [S3 UniChrome Pro] (rev 01) (prog-if 00 [VGA controller]) Subsystem: CLEVO/KAPOK Computer Device 4702 Flags: bus master, 66MHz, medium devsel, latency 64, IRQ 16 Memory at f0000000 (32-bit, prefetchable) [size=64M] Memory at d1000000 (32-bit, non-prefetchable) [size=16M] Expansion ROM at <unassigned> [disabled] Capabilities: [60] Power Management version 2 Capabilities: [70] AGP version 3.0
Signed-off-by: Márton Németh nm127@freemail.hu
Meh, I fail, thanks for catching this. Although the drm/sis driver has the same problem, care to write that patch, too? In any case:
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch Cc: stable@vger.kernel.org
-Daniel
diff --git a/drivers/gpu/drm/via/via_map.c b/drivers/gpu/drm/via/via_map.c index 1f18225..c126182 100644 --- a/drivers/gpu/drm/via/via_map.c +++ b/drivers/gpu/drm/via/via_map.c @@ -100,12 +100,11 @@ int via_driver_load(struct drm_device *dev, unsigned long chipset) if (dev_priv == NULL) return -ENOMEM;
idr_init(&dev_priv->object_idr); dev->dev_private = (void *)dev_priv;
dev_priv->chipset = chipset;
idr_init(&dev->object_name_idr);
pci_set_master(dev->pdev);
ret = drm_vblank_init(dev, 1);
From: Márton Németh nm127@freemail.hu
The filed object_idr of struct drm_sis_private was introduced with commit http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=... .
The idr_init(&dev->object_name_idr) is called instead of idr_init(&dev_priv->object_idr) by mistake, leaving object_idr uninitialized. Correct this.
This patch was not tested because of lack of hardware.
Signed-off-by: Márton Németh nm127@freemail.hu Cc: Daniel Vetter daniel.vetter@ffwll.ch --- diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c index 30d98d1..dd14cd1 100644 --- a/drivers/gpu/drm/sis/sis_drv.c +++ b/drivers/gpu/drm/sis/sis_drv.c @@ -47,9 +47,9 @@ static int sis_driver_load(struct drm_device *dev, unsigned long chipset) if (dev_priv == NULL) return -ENOMEM;
+ idr_init(&dev_priv->object_idr); dev->dev_private = (void *)dev_priv; dev_priv->chipset = chipset; - idr_init(&dev->object_name_idr);
return 0; }
Spotted while reviewing the same patch for drm/via by Németh Márton.
Cc: stable@vger.kernel.org Signed-Off-by: Daniel Vetter daniel.vetter@ffwll.ch --- drivers/gpu/drm/sis/sis_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c index 30d98d1..c7263a4 100644 --- a/drivers/gpu/drm/sis/sis_drv.c +++ b/drivers/gpu/drm/sis/sis_drv.c @@ -49,7 +49,7 @@ static int sis_driver_load(struct drm_device *dev, unsigned long chipset)
dev->dev_private = (void *)dev_priv; dev_priv->chipset = chipset; - idr_init(&dev->object_name_idr); + idr_init(&dev_priv->object_idr);
return 0; }
dri-devel@lists.freedesktop.org