Author: hselasky Date: Fri Jan 17 10:35:18 2014 New Revision: 260814 URL: http://svnweb.freebsd.org/changeset/base/260814
Log: Fix a possible memory use after free and leak situation associated with USB device detach when using character device handles. This also includes LibUSB. It turns out that "usb_close()" cannot always get a reference to clean up its USB transfers and such, if called during the kernel USB device detach. Analysis by: hselasky @ Reported by: Juergen Lock <[email protected]> MFC after: 1 week Modified: head/sys/dev/usb/usb_dev.c head/sys/dev/usb/usb_device.c Modified: head/sys/dev/usb/usb_dev.c ============================================================================== --- head/sys/dev/usb/usb_dev.c Fri Jan 17 10:34:01 2014 (r260813) +++ head/sys/dev/usb/usb_dev.c Fri Jan 17 10:35:18 2014 (r260814) @@ -207,6 +207,11 @@ usb_ref_device(struct usb_cdev_privdata DPRINTFN(2, "no device at %u\n", cpd->dev_index); goto error; } + if (cpd->udev->state == USB_STATE_DETACHED && + (need_uref != 2)) { + DPRINTFN(2, "device is detached\n"); + goto error; + } if (cpd->udev->refcount == USB_DEV_REF_MAX) { DPRINTFN(2, "no dev ref\n"); goto error; @@ -922,23 +927,12 @@ usb_close(void *arg) DPRINTFN(2, "cpd=%p\n", cpd); - err = usb_ref_device(cpd, &refs, 0); - if (err) + err = usb_ref_device(cpd, &refs, + 2 /* uref and allow detached state */); + if (err) { + DPRINTFN(0, "Cannot grab USB reference when " + "closing USB file handle\n"); goto done; - - /* - * If this function is not called directly from the root HUB - * thread, there is usually a need to lock the enumeration - * lock. Check this. - */ - if (!usbd_enum_is_locked(cpd->udev)) { - - DPRINTFN(2, "Locking enumeration\n"); - - /* reference device */ - err = usb_usb_ref_device(cpd, &refs); - if (err) - goto done; } if (cpd->fflags & FREAD) { usb_fifo_close(refs.rxfifo, cpd->fflags); Modified: head/sys/dev/usb/usb_device.c ============================================================================== --- head/sys/dev/usb/usb_device.c Fri Jan 17 10:34:01 2014 (r260813) +++ head/sys/dev/usb/usb_device.c Fri Jan 17 10:35:18 2014 (r260814) @@ -2070,6 +2070,8 @@ usb_free_device(struct usb_device *udev, DPRINTFN(4, "udev=%p port=%d\n", udev, udev->port_no); bus = udev->bus; + + /* set DETACHED state to prevent any further references */ usb_set_device_state(udev, USB_STATE_DETACHED); #if USB_HAVE_DEVCTL @@ -2085,16 +2087,7 @@ usb_free_device(struct usb_device *udev, usb_free_symlink(udev->ugen_symlink); udev->ugen_symlink = NULL; } -#endif - /* - * Unregister our device first which will prevent any further - * references: - */ - usb_bus_port_set_device(bus, udev->parent_hub ? - udev->parent_hub->hub->ports + udev->port_index : NULL, - NULL, USB_ROOT_HUB_ADDR); -#if USB_HAVE_UGEN /* wait for all pending references to go away: */ mtx_lock(&usb_ref_lock); udev->refcount--; @@ -2114,6 +2107,11 @@ usb_free_device(struct usb_device *udev, /* the following will get the device unconfigured in software */ usb_unconfigure(udev, USB_UNCFG_FLAG_FREE_EP0); + /* final device unregister after all character devices are closed */ + usb_bus_port_set_device(bus, udev->parent_hub ? + udev->parent_hub->hub->ports + udev->port_index : NULL, + NULL, USB_ROOT_HUB_ADDR); + /* unsetup any leftover default USB transfers */ usbd_transfer_unsetup(udev->ctrl_xfer, USB_CTRL_XFER_MAX); @@ -2647,8 +2645,14 @@ usb_set_device_state(struct usb_device * DPRINTF("udev %p state %s -> %s\n", udev, usb_statestr(udev->state), usb_statestr(state)); - udev->state = state; +#if USB_HAVE_UGEN + mtx_lock(&usb_ref_lock); +#endif + udev->state = state; +#if USB_HAVE_UGEN + mtx_unlock(&usb_ref_lock); +#endif if (udev->bus->methods->device_state_change != NULL) (udev->bus->methods->device_state_change) (udev); } _______________________________________________ [email protected] mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "[email protected]"
