From: Márton Németh <[email protected]>
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=77ee8f3825054f23b17e9c8f728f061defd86cdc .
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 <[email protected]>
---
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 <[email protected]>
>
> 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=77ee8f3825054f23b17e9c8f728f061defd86cdc .
> 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 <[email protected]>
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 <[email protected]>
Cc: [email protected]
-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);
--
Daniel Vetter
Mail: [email protected]
Mobile: +41 (0)79 365 57 48
From: Márton Németh <[email protected]>
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=6de8a748881f1cd9d795454da2b6db616d5ca3d7 .
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 <[email protected]>
Cc: Daniel Vetter <[email protected]>
---
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;
}