Date: Sat, 21 Jan 2006 21:33:00 +1100 (EST) From: Peter Jeremy <peterjeremy@optushome.com.au> To: FreeBSD-gnats-submit@FreeBSD.org Subject: kern/92092: [PATCH] Panic if device with iicbus child is detached Message-ID: <200601211033.k0LAX0Wf000865@server.vk2pj.dyndns.org> Resent-Message-ID: <200601211040.k0LAe7hY090844@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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@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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200601211033.k0LAX0Wf000865>