Skip to contentSkip to conversation

drm via: initialize object_idr

PATCH In review Németh Márton 3 participants 4 messages Jun 10, 2012 – Jun 11, 2012

Subsystems (2)
DRM drivers DRM misc
lore
Share & tools
lore

Németh Márton

author
#0 lore raw

PATCH 1/1 drm via: initialize object_idr

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]>

---

drivers/gpu/drm/via/via_map.c +1 -2
index 1f18225..c126182 100644
@@ -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);
Reviewed-by on v1 1/1
#1 lore raw
Quoted 26 lines · Németh Márton

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
Quoted 1 line
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

Quoted 19 lines

---

drivers/gpu/drm/via/via_map.c +1 -2
index 1f18225..c126182 100644
@@ -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);
Signature · 3 lines
Daniel Vetter
Mail: [email protected]
Mobile: +41 (0)79 365 57 48
author
#2 lore raw

PATCH 1/1 drm sis: initialize object_idr

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]>

---

drivers/gpu/drm/sis/sis_drv.c +1 -1
index 30d98d1..dd14cd1 100644
@@ -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;
 }
#3 lore raw

PATCH 1/1 drm/sis: properly initialize dev_priv->object_idr

In reply to Németh Márton

Spotted while reviewing the same patch for drm/via by Németh Márton.

Cc [email protected]
Signed-off-by Daniel Vetter <[email protected]>
---
 drivers/gpu/drm/sis/sis_drv.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/sis/sis_drv.c +1 -1
index 30d98d1..c7263a4 100644
@@ -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;
 }
Signature · 6 lines
1.7.10

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel