From 028d9e66a61dae0d4d5b0b1229f3f04c72b3f597 Mon Sep 17 00:00:00 2001 From: hselasky Date: Mon, 7 Nov 2016 09:27:05 +0000 Subject: [PATCH] MFC r307518: Fix device delete child function. When detaching device trees parent devices must be detached prior to detaching its children. This is because parent devices can have pointers to the child devices in their softcs which are not invalidated by device_delete_child(). This can cause use after free issues and panic(). Device drivers implementing trees, must ensure its detach function detaches or deletes all its children before returning. While at it remove now redundant device_detach() calls before device_delete_child() and device_delete_children(), mostly in the USB controller drivers. Tested by: Jan Henrik Sylvester Reviewed by: jhb Differential Revision: https://reviews.freebsd.org/D8070 git-svn-id: svn://svn.freebsd.org/base/stable/8@308404 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f --- sys/dev/puc/puc.c | 3 +-- sys/dev/usb/controller/at91dci_atmelarm.c | 6 ------ sys/dev/usb/controller/atmegadci_atmelarm.c | 6 ------ sys/dev/usb/controller/ehci_ixp4xx.c | 6 ------ sys/dev/usb/controller/ehci_pci.c | 6 ------ sys/dev/usb/controller/musb_otg_atmelarm.c | 6 ------ sys/dev/usb/controller/ohci_pci.c | 6 ------ sys/dev/usb/controller/uhci_pci.c | 6 ------ sys/dev/usb/controller/uss820dci_atmelarm.c | 6 ------ sys/dev/usb/controller/xhci_pci.c | 6 ------ sys/dev/usb/usb_device.c | 4 +--- sys/kern/subr_bus.c | 8 +++++--- sys/mips/atheros/ar71xx_ehci.c | 6 ------ sys/mips/atheros/ar71xx_ohci.c | 6 ------ sys/mips/cavium/usb/octusb_octeon.c | 6 ------ sys/mips/rmi/xls_ehci.c | 6 ------ 16 files changed, 7 insertions(+), 86 deletions(-) diff --git a/sys/dev/puc/puc.c b/sys/dev/puc/puc.c index cad3a0b99..696bd248e 100644 --- a/sys/dev/puc/puc.c +++ b/sys/dev/puc/puc.c @@ -412,8 +412,7 @@ puc_bfe_detach(device_t dev) port = &sc->sc_port[idx]; if (port->p_dev == NULL) continue; - if (device_detach(port->p_dev) == 0) { - device_delete_child(dev, port->p_dev); + if (device_delete_child(dev, port->p_dev) == 0) { if (port->p_rres != NULL) rman_release_resource(port->p_rres); if (port->p_ires != NULL) diff --git a/sys/dev/usb/controller/at91dci_atmelarm.c b/sys/dev/usb/controller/at91dci_atmelarm.c index 4b2a03945..dfc3d0f52 100644 --- a/sys/dev/usb/controller/at91dci_atmelarm.c +++ b/sys/dev/usb/controller/at91dci_atmelarm.c @@ -261,14 +261,8 @@ static int at91_udp_detach(device_t dev) { struct at91_udp_softc *sc = device_get_softc(dev); - device_t bdev; int err; - if (sc->sc_dci.sc_bus.bdev) { - bdev = sc->sc_dci.sc_bus.bdev; - device_detach(bdev); - device_delete_child(dev, bdev); - } /* during module unload there are lots of children leftover */ device_delete_all_children(dev); diff --git a/sys/dev/usb/controller/atmegadci_atmelarm.c b/sys/dev/usb/controller/atmegadci_atmelarm.c index 1bc7e6815..e70bd49c3 100644 --- a/sys/dev/usb/controller/atmegadci_atmelarm.c +++ b/sys/dev/usb/controller/atmegadci_atmelarm.c @@ -154,14 +154,8 @@ static int atmegadci_detach(device_t dev) { struct atmegadci_super_softc *sc = device_get_softc(dev); - device_t bdev; int err; - if (sc->sc_otg.sc_bus.bdev) { - bdev = sc->sc_otg.sc_bus.bdev; - device_detach(bdev); - device_delete_child(dev, bdev); - } /* during module unload there are lots of children leftover */ device_delete_all_children(dev); diff --git a/sys/dev/usb/controller/ehci_ixp4xx.c b/sys/dev/usb/controller/ehci_ixp4xx.c index f6b212311..caf4d7049 100644 --- a/sys/dev/usb/controller/ehci_ixp4xx.c +++ b/sys/dev/usb/controller/ehci_ixp4xx.c @@ -206,14 +206,8 @@ ehci_ixp_detach(device_t self) { struct ixp_ehci_softc *isc = device_get_softc(self); ehci_softc_t *sc = &isc->base; - device_t bdev; int err; - if (sc->sc_bus.bdev) { - bdev = sc->sc_bus.bdev; - device_detach(bdev); - device_delete_child(self, bdev); - } /* during module unload there are lots of children leftover */ device_delete_all_children(self); diff --git a/sys/dev/usb/controller/ehci_pci.c b/sys/dev/usb/controller/ehci_pci.c index 874659c2f..4f958061f 100644 --- a/sys/dev/usb/controller/ehci_pci.c +++ b/sys/dev/usb/controller/ehci_pci.c @@ -458,13 +458,7 @@ static int ehci_pci_detach(device_t self) { ehci_softc_t *sc = device_get_softc(self); - device_t bdev; - if (sc->sc_bus.bdev) { - bdev = sc->sc_bus.bdev; - device_detach(bdev); - device_delete_child(self, bdev); - } /* during module unload there are lots of children leftover */ device_delete_all_children(self); diff --git a/sys/dev/usb/controller/musb_otg_atmelarm.c b/sys/dev/usb/controller/musb_otg_atmelarm.c index 988ef377c..84e8474c4 100644 --- a/sys/dev/usb/controller/musb_otg_atmelarm.c +++ b/sys/dev/usb/controller/musb_otg_atmelarm.c @@ -179,14 +179,8 @@ static int musbotg_detach(device_t dev) { struct musbotg_super_softc *sc = device_get_softc(dev); - device_t bdev; int err; - if (sc->sc_otg.sc_bus.bdev) { - bdev = sc->sc_otg.sc_bus.bdev; - device_detach(bdev); - device_delete_child(dev, bdev); - } /* during module unload there are lots of children leftover */ device_delete_all_children(dev); diff --git a/sys/dev/usb/controller/ohci_pci.c b/sys/dev/usb/controller/ohci_pci.c index bfcc2c2f3..ec5274558 100644 --- a/sys/dev/usb/controller/ohci_pci.c +++ b/sys/dev/usb/controller/ohci_pci.c @@ -329,13 +329,7 @@ static int ohci_pci_detach(device_t self) { ohci_softc_t *sc = device_get_softc(self); - device_t bdev; - if (sc->sc_bus.bdev) { - bdev = sc->sc_bus.bdev; - device_detach(bdev); - device_delete_child(self, bdev); - } /* during module unload there are lots of children leftover */ device_delete_all_children(self); diff --git a/sys/dev/usb/controller/uhci_pci.c b/sys/dev/usb/controller/uhci_pci.c index 4af440bfb..fbd9c8c0f 100644 --- a/sys/dev/usb/controller/uhci_pci.c +++ b/sys/dev/usb/controller/uhci_pci.c @@ -379,13 +379,7 @@ int uhci_pci_detach(device_t self) { uhci_softc_t *sc = device_get_softc(self); - device_t bdev; - if (sc->sc_bus.bdev) { - bdev = sc->sc_bus.bdev; - device_detach(bdev); - device_delete_child(self, bdev); - } /* during module unload there are lots of children leftover */ device_delete_all_children(self); diff --git a/sys/dev/usb/controller/uss820dci_atmelarm.c b/sys/dev/usb/controller/uss820dci_atmelarm.c index 55fb166c8..449311289 100644 --- a/sys/dev/usb/controller/uss820dci_atmelarm.c +++ b/sys/dev/usb/controller/uss820dci_atmelarm.c @@ -168,14 +168,8 @@ static int uss820_atmelarm_detach(device_t dev) { struct uss820dci_softc *sc = device_get_softc(dev); - device_t bdev; int err; - if (sc->sc_bus.bdev) { - bdev = sc->sc_bus.bdev; - device_detach(bdev); - device_delete_child(dev, bdev); - } /* during module unload there are lots of children leftover */ device_delete_all_children(dev); diff --git a/sys/dev/usb/controller/xhci_pci.c b/sys/dev/usb/controller/xhci_pci.c index 895e90070..f0692c4a3 100644 --- a/sys/dev/usb/controller/xhci_pci.c +++ b/sys/dev/usb/controller/xhci_pci.c @@ -294,13 +294,7 @@ static int xhci_pci_detach(device_t self) { struct xhci_softc *sc = device_get_softc(self); - device_t bdev; - if (sc->sc_bus.bdev != NULL) { - bdev = sc->sc_bus.bdev; - device_detach(bdev); - device_delete_child(self, bdev); - } /* during module unload there are lots of children leftover */ device_delete_all_children(self); diff --git a/sys/dev/usb/usb_device.c b/sys/dev/usb/usb_device.c index de30122b3..8ff0c4912 100644 --- a/sys/dev/usb/usb_device.c +++ b/sys/dev/usb/usb_device.c @@ -1074,10 +1074,8 @@ usb_detach_device_sub(struct usb_device *udev, device_t *ppdev, device_printf(dev, "Resume failed\n"); } } - if (device_detach(dev)) { - goto error; - } } + /* detach and delete child */ if (device_delete_child(udev->parent_dev, dev)) { goto error; } diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c index 6162b6f23..7ea5cfce1 100644 --- a/sys/kern/subr_bus.c +++ b/sys/kern/subr_bus.c @@ -1896,15 +1896,17 @@ device_delete_child(device_t dev, device_t child) PDEBUG(("%s from %s", DEVICENAME(child), DEVICENAME(dev))); - /* remove children first */ + /* detach parent before deleting children, if any */ + if ((error = device_detach(child)) != 0) + return (error); + + /* remove children second */ while ((grandchild = TAILQ_FIRST(&child->children)) != NULL) { error = device_delete_child(child, grandchild); if (error) return (error); } - if ((error = device_detach(child)) != 0) - return (error); if (child->devclass) devclass_delete_device(child->devclass, child); TAILQ_REMOVE(&dev->children, child, link); diff --git a/sys/mips/atheros/ar71xx_ehci.c b/sys/mips/atheros/ar71xx_ehci.c index 129b0c17c..2e0082eee 100644 --- a/sys/mips/atheros/ar71xx_ehci.c +++ b/sys/mips/atheros/ar71xx_ehci.c @@ -172,14 +172,8 @@ ar71xx_ehci_detach(device_t self) { struct ar71xx_ehci_softc *isc = device_get_softc(self); ehci_softc_t *sc = &isc->base; - device_t bdev; int err; - if (sc->sc_bus.bdev) { - bdev = sc->sc_bus.bdev; - device_detach(bdev); - device_delete_child(self, bdev); - } /* during module unload there are lots of children leftover */ device_delete_all_children(self); diff --git a/sys/mips/atheros/ar71xx_ohci.c b/sys/mips/atheros/ar71xx_ohci.c index 51c25cd65..00fb6d7ab 100644 --- a/sys/mips/atheros/ar71xx_ohci.c +++ b/sys/mips/atheros/ar71xx_ohci.c @@ -144,13 +144,7 @@ static int ar71xx_ohci_detach(device_t dev) { struct ar71xx_ohci_softc *sc = device_get_softc(dev); - device_t bdev; - if (sc->sc_ohci.sc_bus.bdev) { - bdev = sc->sc_ohci.sc_bus.bdev; - device_detach(bdev); - device_delete_child(dev, bdev); - } /* during module unload there are lots of children leftover */ device_delete_all_children(dev); diff --git a/sys/mips/cavium/usb/octusb_octeon.c b/sys/mips/cavium/usb/octusb_octeon.c index 65208cc9a..31a073aa6 100644 --- a/sys/mips/cavium/usb/octusb_octeon.c +++ b/sys/mips/cavium/usb/octusb_octeon.c @@ -150,14 +150,8 @@ static int octusb_octeon_detach(device_t dev) { struct octusb_octeon_softc *sc = device_get_softc(dev); - device_t bdev; int err; - if (sc->sc_dci.sc_bus.bdev) { - bdev = sc->sc_dci.sc_bus.bdev; - device_detach(bdev); - device_delete_child(dev, bdev); - } /* during module unload there are lots of children leftover */ device_delete_all_children(dev); diff --git a/sys/mips/rmi/xls_ehci.c b/sys/mips/rmi/xls_ehci.c index 2b6faff48..474c0c0c0 100644 --- a/sys/mips/rmi/xls_ehci.c +++ b/sys/mips/rmi/xls_ehci.c @@ -170,14 +170,8 @@ static int ehci_xls_detach(device_t self) { ehci_softc_t *sc = device_get_softc(self); - device_t bdev; int err; - if (sc->sc_bus.bdev) { - bdev = sc->sc_bus.bdev; - device_detach(bdev); - device_delete_child(self, bdev); - } /* during module unload there are lots of children leftover */ device_delete_all_children(self); -- 2.42.0