From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id AF2E4C4332F for ; Tue, 18 Oct 2022 14:46:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230234AbiJROqh (ORCPT ); Tue, 18 Oct 2022 10:46:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59448 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230150AbiJROqg (ORCPT ); Tue, 18 Oct 2022 10:46:36 -0400 Received: from netrider.rowland.org (netrider.rowland.org [192.131.102.5]) by lindbergh.monkeyblade.net (Postfix) with SMTP id CAF67D73DD for ; Tue, 18 Oct 2022 07:46:34 -0700 (PDT) Received: (qmail 1214465 invoked by uid 1000); 18 Oct 2022 10:46:34 -0400 Date: Tue, 18 Oct 2022 10:46:34 -0400 From: Alan Stern To: Ricardo Ribalda Cc: inux@roeck-us.net, Nazar Mokrynskyi , Laurent Pinchart , linux-media@vger.kernel.org, linux-usb@vger.kernel.org Subject: Re: [Bug 216543] kernel NULL pointer dereference usb_hcd_alloc_bandwidth Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org On Tue, Oct 18, 2022 at 02:40:44PM +0900, Ricardo Ribalda wrote: > Hi > > Guenter already provided some patches to fix this issue: > https://lore.kernel.org/lkml/20200917022547.198090-1-linux@roeck-us.net/ > > Until we have a solution on the core (or rewrite the kernel in rust > ;P) , I think we should merge them (or something similar). > > I can prepare a patchset merging Guenter set and my "grannular PM" > https://lore.kernel.org/linux-media/20220920-resend-powersave-v1-0-123aa2ba3836@chromium.org/ > > It can always be reverted when we reach consensus on how to do it for > every driver. > > Regards! I was going to say "Wow! Quick work, thank you!" until I noticed that the patch set was submitted over two years ago and still hasn't been merged. :-( Well, Nazar, at least now you have an answer and a potential fix. Alan Stern > On Tue, 18 Oct 2022 at 06:46, Alan Stern wrote: > > > > Moving this bug report from bugzilla to the mailing lists. > > > > The short description of the bug is that in uvcvideo, disconnect races > > with starting a video transfer. The race shows up on Nazar's system > > because of a marginal USB cable which leads to a lot of spontaneous > > disconnections. > > > > On Mon, Oct 17, 2022 at 05:59:48PM +0000, bugzilla-daemon@kernel.org wrote: > > > https://bugzilla.kernel.org/show_bug.cgi?id=216543 > > > > > > --- Comment #7 from Nazar Mokrynskyi (nazar@mokrynskyi.com) --- > > > Created attachment 303022 > > > --> https://bugzilla.kernel.org/attachment.cgi?id=303022&action=edit > > > Kernel log with uvc-trace patch applied > > > > For everyone's information, here is the uvc-trace patch. All it does is > > add messages to the kernel log when uvcvideo's probe and disconnect > > routines run, and just before uvc_video_start_transfer() calls > > usb_set_interface(). > > > > --- usb-devel/drivers/media/usb/uvc/uvc_video.c > > +++ usb-devel/drivers/media/usb/uvc/uvc_video.c > > @@ -1965,6 +1965,7 @@ static int uvc_video_start_transfer(stru > > "Selecting alternate setting %u (%u B/frame bandwidth)\n", > > altsetting, best_psize); > > > > + dev_info(&intf->dev, "uvc set alt\n"); > > ret = usb_set_interface(stream->dev->udev, intfnum, altsetting); > > if (ret < 0) > > return ret; > > --- usb-devel/drivers/media/usb/uvc/uvc_driver.c > > +++ usb-devel/drivers/media/usb/uvc/uvc_driver.c > > @@ -2374,6 +2374,8 @@ static int uvc_probe(struct usb_interfac > > int function; > > int ret; > > > > + dev_info(&intf->dev, "uvc_probe start\n"); > > + > > /* Allocate memory for the device and initialize it. */ > > dev = kzalloc(sizeof(*dev), GFP_KERNEL); > > if (dev == NULL) > > @@ -2535,6 +2537,7 @@ static void uvc_disconnect(struct usb_in > > return; > > > > uvc_unregister_video(dev); > > + dev_info(&intf->dev, "uvc_disconnect done\n"); > > kref_put(&dev->ref, uvc_delete); > > } > > > > The output in the kernel log below clearly shows that there is a bug in > > the uvcvideo driver. > > > > > I'm on 6.0.2 and seemingly get this even more frequently with good cable and no > > > extra adapters. So I patched 6.0.2 with uvc-trace above and reproduced it > > > within a few minutes. > > > > > > USB seems to reset, often camera stops or freezes in the browser, but the light > > > on the camera itself remains on. Sometimes I can enable/disable/enable camera > > > for it to reboot, but the last time I did that in the log I got null pointer > > > de-reference again. > > > > Here is the important part of the log: > > > > [ 684.746848] usb 8-2.4.4: reset SuperSpeed USB device number 6 using xhci_hcd > > [ 684.810979] uvcvideo 8-2.4.4:1.0: uvc_probe start > > [ 684.811032] usb 8-2.4.4: Found UVC 1.00 device Logitech BRIO (046d:085e) > > [ 684.843413] input: Logitech BRIO as /devices/pci0000:00/0000:00:08.1/0000:59:00.3/usb8/8-2/8-2.4/8-2.4.4/8-2.4.4:1.0/input/input43 > > [ 684.911255] usb 8-2.4.4: current rate 16000 is different from the runtime rate 24000 > > ... > > [ 743.800368] uvcvideo 8-2.4.4:1.1: uvc set alt > > > > This is where an ioctl calls uvc_video_start_transfer. > > > > [ 748.654701] usb 8-2.4.4: USB disconnect, device number 6 > > [ 748.714355] uvcvideo 8-2.4.4:1.0: uvc_disconnect done > > > > This is where the disconnect starts and finishes > > > > [ 748.898340] BUG: kernel NULL pointer dereference, address: 0000000000000000 > > [ 748.898344] #PF: supervisor read access in kernel mode > > [ 748.898346] #PF: error_code(0x0000) - not-present page > > [ 748.898347] PGD 0 P4D 0 > > [ 748.898349] Oops: 0000 [#1] PREEMPT SMP NOPTI > > [ 748.898351] CPU: 16 PID: 11890 Comm: VideoCapture Not tainted 6.0.2-x64v2-uvc-trace-xanmod1 #1 > > [ 748.898353] Hardware name: Gigabyte Technology Co., Ltd. B550 VISION D/B550 VISION D, BIOS F15d 07/20/2022 > > [ 748.898354] RIP: 0010:usb_ifnum_to_if+0x35/0x60 > > ... > > [ 748.898368] Call Trace: > > [ 748.898370] > > [ 748.898370] usb_hcd_alloc_bandwidth+0x240/0x370 > > [ 748.898375] usb_set_interface+0x122/0x350 > > [ 748.898378] uvc_video_start_transfer.cold+0xd8/0x2ae [uvcvideo] > > [ 748.898383] uvc_video_start_streaming+0x75/0xd0 [uvcvideo] > > [ 748.898386] uvc_start_streaming+0x25/0xe0 [uvcvideo] > > [ 748.898390] vb2_start_streaming+0x86/0x140 [videobuf2_common] > > [ 748.898393] vb2_core_streamon+0x57/0xc0 [videobuf2_common] > > [ 748.898395] uvc_queue_streamon+0x25/0x40 [uvcvideo] > > [ 748.898398] uvc_ioctl_streamon+0x35/0x60 [uvcvideo] > > [ 748.898401] __video_do_ioctl+0x19a/0x3f0 [videodev] > > > > And this proves that uvc_disconnect() returned before the driver was > > finished accessing the device. > > > > I don't know how the driver works or how it tries to prevent this sort > > of race from occurring, but apparently the strategy isn't working. > > > > > Please let me know if there is any other information I can provide and what > > > could be the root cause of this annoying behavior. > > > > At this point I will bow out of the discussion; it's up to the uvcvideo > > maintainers to investigate further. Maybe they can provide a patch for > > you to test. > > > > Alan Stern > > > > -- > Ricardo Ribalda