kern/92092: [PATCH] Panic if device with iicbus child is detached

[Peter Jeremy]

>Number:         92092
>Category:       kern
>Synopsis:       [PATCH] Panic if device with iicbus child is detached
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sat Jan 21 10:40:07 GMT 2006
>Closed-Date:
>Last-Modified:
>Originator:     Peter Jeremy
>Release:        FreeBSD 7.0-CURRENT i386
>Organization:
n/a
>Environment:
System: FreeBSD server.vk2pj.dyndns.org 7.0-CURRENT FreeBSD 7.0-CURRENT #24: Sat Jan 21 20:55:05 EST 2006 root at server.vk2pj.dyndns.org:/var/obj/k7/usr/src/sys/server i386

>Description:

When deleting a device (using device_delete_child()), all its child
devices are recursively deleted and then device_detach() is called.

In the case of iicsmb, this causes a panic because iicsmb_detach()
also calls device_delete_child() on the smbus child it has cached in
its softc.  By this time that child has been destroyed.

Looking at device_detach(), it appears that the solution is to create
a bus_child_detached method to inform the parent that a child is being
destroyed so it can invalidate cached pointers to that child.

Looking at the iicsmb code, it appears that iicbb and iicsmb both
cache and explicitly delete child devices and therefore both need
bus_child_detached methods.

Looking at the number of different devices attached as children,
together with the paucity of bus_child_detached methods (only isa, ed,
pccard, cbb and usb exist), there are likely to be other similar
issues elsewhere.

>How-To-Repeat:
	Create a device that does
		child = device_add_child(dev, "iicbus", -1);
	on attach and
		device_delete_child(dev, child);
	on detach.  Attach and detach the device (eg load/unload it as a kld)
	with UMA memory debugging enabled (0xdeadc0de fill on free).
	You should get a panic that looks similar to:

--- trap 0xc, eip = 0xc056229a, esp = 0xd6b9cae4, ebp = 0xd6b9caf0 ---
device_delete_child(c32e6900,deadc0de,c3379610,d6b9cb1c,c04a1fb6) at device_delete_child+0xa
device_delete_child(c3350100,c32e6900,c3350100,c3350100,d6b9cb40) at device_delete_child+0x1d
iicsmb_detach(c3350100,c3321050,c0748228,965,c3350100) at iicsmb_detach+0x36
device_detach(c3350100,c32e6900,c336e100,d6b9cb68,c05622ad) at device_detach+0x8e
device_delete_child(c336e100,c3350100,c338f000,d6b9cb88,c087fd7f) at device_delete_child+0x30
device_delete_child(c3371180,c336e100,0,c33a2e00,c3371180) at device_delete_child+0x1d
release_resources(c338f000,c0886928,c3371180,c3371180,d6b9cbc4) at release_resources+0xdf
saa_detach(c3371180,c3331850,c0748228,965,c331e9a0) at saa_detach+0x84
device_detach(c3371180,c0885188,c3371180,c32f0380,c0886940) at device_detach+0x8e
devclass_delete_driver(c32f0380,c0886928,1,c3281c80,c3281c80) at devclass_delete_driver+0x95
driver_module_handler(c3281c80,1,c0886940) at driver_module_handler+0xf3
module_unload(c3281c80,0,1fb,0,0) at module_unload+0x60
linker_file_unload(c32a2e00,0,c0707925,327,2) at linker_file_unload+0x87
kern_kldunload(c3a30600,2,0,d6b9cd30,c06c3d63) at kern_kldunload+0x94
kldunloadf(c3a30600,d6b9cd04,8,440,c3b6b318) at kldunloadf+0x2c

>Fix:

Index: sys/dev/iicbus/iicbb.c
===================================================================
RCS file: /usr/ncvs/src/sys/dev/iicbus/iicbb.c,v
retrieving revision 1.13
diff -u -r1.13 iicbb.c
--- sys/dev/iicbus/iicbb.c	24 Aug 2003 17:49:13 -0000	1.13
+++ sys/dev/iicbus/iicbb.c	21 Jan 2006 10:15:38 -0000
@@ -67,6 +67,7 @@
 static int iicbb_attach(device_t);
 static int iicbb_detach(device_t);
 static int iicbb_print_child(device_t, device_t);
+static void iicbb_child_detached(device_t dev, device_t child);
 
 static int iicbb_callback(device_t, int, caddr_t);
 static int iicbb_start(device_t, u_char, int);
@@ -83,6 +84,7 @@
 
 	/* bus interface */
 	DEVMETHOD(bus_print_child,	iicbb_print_child),
+	DEVMETHOD(bus_child_detached,	iicbb_child_detached),
 
 	/* iicbus interface */
 	DEVMETHOD(iicbus_callback,	iicbb_callback),
@@ -139,6 +141,16 @@
 	return (0);
 }
 
+static void
+iicbb_child_detached(device_t dev, device_t child)
+{
+	struct iicbb_softc *sc = (struct iicbb_softc *)device_get_softc(dev);
+
+	if (child == sc->iicbus)
+		sc->iicbus = NULL;
+	/* need to abort any operation in progress as well */
+}
+
 static int
 iicbb_print_child(device_t bus, device_t dev)
 {
Index: sys/dev/iicbus/iicsmb.c
===================================================================
RCS file: /usr/ncvs/src/sys/dev/iicbus/iicsmb.c,v
retrieving revision 1.12
diff -u -r1.12 iicsmb.c
--- sys/dev/iicbus/iicsmb.c	10 Aug 2003 14:28:24 -0000	1.12
+++ sys/dev/iicbus/iicsmb.c	21 Jan 2006 10:15:53 -0000
@@ -81,6 +81,7 @@
 static int iicsmb_attach(device_t);
 static int iicsmb_detach(device_t);
 static void iicsmb_identify(driver_t *driver, device_t parent);
+static void iicsmb_child_detached(device_t dev, device_t child);
 
 static void iicsmb_intr(device_t dev, int event, char *buf);
 static int iicsmb_callback(device_t dev, int index, caddr_t data);
@@ -107,6 +108,7 @@
 	/* bus interface */
 	DEVMETHOD(bus_driver_added,	bus_generic_driver_added),
 	DEVMETHOD(bus_print_child,	bus_generic_print_child),
+	DEVMETHOD(bus_child_detached,	iicsmb_child_detached),
 
 	/* iicbus interface */
 	DEVMETHOD(iicbus_intr,		iicsmb_intr),
@@ -176,6 +178,16 @@
 	return (0);
 }
 
+static void
+iicsmb_child_detached(device_t dev, device_t child)
+{
+	struct iicsmb_softc *sc = (struct iicsmb_softc *)device_get_softc(dev);
+
+	if (child == sc->smbus)
+		sc->smbus = NULL;
+	/* need to abort any operation in progress as well */
+}
+
 /*
  * iicsmb_intr()
  *


>Release-Note:
>Audit-Trail:
>Unformatted: