d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix a possible memory use after free and leak situation associated

Browse Source 
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 <nox@jelal.kn-bremen.de>
MFC after:	1 week
This commit is contained in:
Hans Petter Selasky 2014-01-17 10:35:18 +00:00
parent 6d03ca5789
commit f97da128ab
2 changed files with 24 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
sys/dev/usb/usb_dev.c
View File

@ -207,6 +207,11 @@ usb_ref_device(struct usb_cdev_privdata *cpd,
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);

24
sys/dev/usb/usb_device.c
View File

@ -2070,6 +2070,8 @@ usb_free_device(struct usb_device *udev, uint8_t flag)
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, uint8_t flag)
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, uint8_t flag)
/* 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 *udev, enum usb_dev_state state)
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);
}