Bug 207871
Summary: | nullpointer dereference in uvc_video_stop_streaming | ||
---|---|---|---|
Product: | Drivers | Reporter: | Tobias Diedrich (ranma+kernel) |
Component: | USB | Assignee: | Default virtual assignee for Drivers/USB (drivers_usb) |
Status: | NEW --- | ||
Severity: | normal | CC: | stern |
Priority: | P1 | ||
Hardware: | x86-64 | ||
OS: | Linux | ||
Kernel Version: | 5.4.26 | Subsystem: | |
Regression: | No | Bisected commit-id: |
Description
Tobias Diedrich
2020-05-23 19:28:10 UTC
Older report for USB2 webcam with same stack backtrace: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1827452 Interestingly, usb_set_interface() already calls usb_ifnum_to_if() once, which did not fail. Then after calling usb_disable_interface() it calls usb_hcd_alloc_bandwidth(), which does another call to usb_ifnum_to_if(), which faulted with the null deref. The fault is at: mov rax, [rdx + 0x398] test rax, rax jz early_exit_since_config_is_null [...] mov rcx, [rax + rdx*8 + 0x98] inc rdx -> mov r8, [rcx] movzx r8d, byte ptr [r8 + 2] [...] So I think the fault is at [...] for (i = 0; i < config->desc.bNumInterfaces; i++) -> if (config->interface[i]->altsetting[0].desc.bInterfaceNumber == ifnum) ^^^^^^^^^^^^nullptr return config->interface[i]; [...] usb_set_interface() only checks for dev->state == USB_STATE_SUSPENDED, maybe it also needs to check for dev->state == USB_STATE_NOTATTACHED? The disconnect message indicates this would have been the state the device was in, from usb_disconnect(): [...] usb_set_device_state(udev, USB_STATE_NOTATTACHED); dev_info(&udev->dev, "USB disconnect, device number %d\n", udev->devnum); [...] The problem is not that usb_set_interface() does inadequate checking; the problem is that the uvc_video driver calls usb_set_interface() at an inappropriate time -- i.e., after its remove routine has returned. (I don't know this for certain, but it seems likely given the context of the bug.) I'm guessing that's the device disconnect callback? static void uvc_disconnect(struct usb_interface *intf) { struct uvc_device *dev = usb_get_intfdata(intf); /* Set the USB interface data to NULL. This can be done outside the * lock, as there's no other reader. */ usb_set_intfdata(intf, NULL); if (intf->cur_altsetting->desc.bInterfaceSubClass == UVC_SC_VIDEOSTREAMING) return; uvc_unregister_video(dev); kref_put(&dev->ref, uvc_delete); } Meanwhile uvc_stop_streaming is a vb2_ops (videobuf2) callback from the V4L2CaptureThread. I'm guessing then that uvc_disconnect() should wait for videobufs to be stopped before returning? Yes. I'm not familiar with this driver, but it looks like uvc_unregister_video() (or one of the routines it calls) is supposed to do the waiting. |