linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* NULL dereference on disconnection during usb_set_interface()
@ 2024-01-21 17:18 Michał Pecio
  2024-02-17 15:31 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 5+ messages in thread
From: Michał Pecio @ 2024-01-21 17:18 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-usb

Hi,

I encountered an interesting race. This USB camera appears to have got
stuck and dropped by the bus during video stream initialization, leading 
to an oops. This is a one time occurrence, not reproducible. Maybe not
a very severe issue due to narrow window of opportunity, but still...

Linux v6.7, the host is XHCI.

The first message below comes from uvc_video_start_transfer(). It is
meant to be followed immediately by a call to usb_set_interface() and
apparently during this call things went weird.

After a half second delay the device was disconnected and another five
seconds later a NULL pointer dereference occured.

The crashing function is usb_ifnum_to_if() and disassembly suggests that
the dereferenced NULL value was config->interface[i], for unknown i.

Thanks,
Michal


[ 7079.664238] usb 9-2: Selecting alternate setting 9 (20480 B/frame bandwidth)
[ 7080.202494] usb 9-2: USB disconnect, device number 2
[ 7085.158804] BUG: kernel NULL pointer dereference, address: 0000000000000000
[ 7085.158814] #PF: supervisor read access in kernel mode
[ 7085.158816] #PF: error_code(0x0000) - not-present page
[ 7085.158818] PGD 0 P4D 0 
[ 7085.158822] Oops: 0000 [#1] PREEMPT SMP
[ 7085.158825] CPU: 0 PID: 12833 Comm: yavta Not tainted 6.7.0 #3
[ 7085.158829] Hardware name: MICRO-STAR INTERNATIONAL CO.,LTD MS-7596/760GM -E51 (MS-7596), BIOS V1.10 02/28/2011
[ 7085.158832] RIP: 0010:usb_ifnum_to_if+0x38/0x50
[ 7085.158839] Code: d2 74 32 0f b6 4a 04 84 c9 74 2e ff c9 48 8d 82 98 00 00 00 48 8d bc ca a0 00 00 00 eb 09 48 83 c0 08 48 39 f8 74 12 48 8b 10 <48> 8b 0a 0f b6 49 02 39 f1 75 e9 48 89 d0 c3 31 d2 48 89 d0 c3 0f
[ 7085.158842] RSP: 0018:ffffc90000d1fba0 EFLAGS: 00010202
[ 7085.158845] RAX: ffff88812deb5898 RBX: ffff88812d78e000 RCX: 0000000000000002
[ 7085.158847] RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffff88812deb58b0
[ 7085.158849] RBP: 0000000000000000 R08: ffffffff826dad88 R09: ffffffff826dad88
[ 7085.158850] R10: 0000000000000400 R11: 0000000000000000 R12: ffff88812ddfd570
[ 7085.158852] R13: 00000000ffffff92 R14: ffff88812ddfd408 R15: ffff88812ddb7000
[ 7085.158854] FS:  00007f4d672ab740(0000) GS:ffff88820fe00000(0000) knlGS:0000000000000000
[ 7085.158856] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 7085.158858] CR2: 0000000000000000 CR3: 000000014c0f5000 CR4: 00000000000006f0
[ 7085.158860] Call Trace:
[ 7085.158863]  <TASK>
[ 7085.158867]  ? __die+0x2d/0x80
[ 7085.158870]  ? page_fault_oops+0x15d/0x420
[ 7085.158874]  ? fixup_exception+0x36/0x280
[ 7085.158879]  ? exc_page_fault+0x74/0x150
[ 7085.158882]  ? asm_exc_page_fault+0x22/0x30
[ 7085.158887]  ? usb_ifnum_to_if+0x38/0x50
[ 7085.158890]  usb_hcd_alloc_bandwidth+0x208/0x310
[ 7085.158895]  usb_set_interface+0x128/0x400
[ 7085.158899]  uvc_video_start_transfer+0x1c4/0x600 [uvcvideo]
[ 7085.158908]  uvc_video_start_streaming+0x79/0xc0 [uvcvideo]
[ 7085.158914]  uvc_start_streaming+0x41/0x100 [uvcvideo]
[ 7085.158922]  vb2_start_streaming+0x60/0x120 [videobuf2_common]
[ 7085.158928]  vb2_core_streamon+0xc2/0x160 [videobuf2_common]
[ 7085.158934]  uvc_queue_streamon+0x35/0x60 [uvcvideo]
[ 7085.158940]  uvc_ioctl_streamon+0x46/0x70 [uvcvideo]
[ 7085.158947]  __video_do_ioctl+0x38a/0x460 [videodev]
[ 7085.158957]  video_usercopy+0x26c/0x720 [videodev]
[ 7085.158965]  ? v4l_prepare_buf+0x80/0x80 [videodev]
[ 7085.158974]  v4l2_ioctl+0x45/0x50 [videodev]
[ 7085.158982]  __x64_sys_ioctl+0xae/0xd0
[ 7085.158987]  ? exit_to_user_mode_prepare+0x7a/0x120
[ 7085.158991]  do_syscall_64+0x2c/0xd0
[ 7085.158994]  entry_SYSCALL_64_after_hwframe+0x46/0x4e
[ 7085.158998] RIP: 0033:0x7f4d673ba3af
[ 7085.159002] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 18 48 8b 44 24 18 64 48 2b 04 25 28 00 00
[ 7085.159004] RSP: 002b:00007ffeecff6480 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[ 7085.159007] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f4d673ba3af
[ 7085.159009] RDX: 00007ffeecff64e4 RSI: 0000000040045612 RDI: 0000000000000003
[ 7085.159010] RBP: 0000000000000001 R08: 0000000000000078 R09: 000055eca2bc36b0
[ 7085.159012] R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffeecff6640
[ 7085.159014] R13: 000055eca2bc3a68 R14: 00000000003a9800 R15: 00000000003a9800
[ 7085.159017]  </TASK>
[ 7085.159018] Modules linked in: xhci_pci xhci_hcd uvcvideo ccm uvc videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videodev videobuf2_common ext2 ath5k mac80211 libarc4 ath serio_raw cfg80211 snd_pcsp dm_mod nfnetlink ip_tables x_tables [last unloaded: xhci_hcd]
[ 7085.159036] CR2: 0000000000000000
[ 7085.159038] ---[ end trace 0000000000000000 ]---


Disassembly:

0000000000000380 <usb_ifnum_to_if>:
 380:   e8 00 00 00 00          call   385 <usb_ifnum_to_if+0x5>
 385:   48 8b 97 a8 03 00 00    mov    0x3a8(%rdi),%rdx
 38c:   48 85 d2                test   %rdx,%rdx
 38f:   74 32                   je     3c3 <usb_ifnum_to_if+0x43>
 391:   0f b6 4a 04             movzbl 0x4(%rdx),%ecx
 395:   84 c9                   test   %cl,%cl
 397:   74 2e                   je     3c7 <usb_ifnum_to_if+0x47>
 399:   ff c9                   dec    %ecx
 39b:   48 8d 82 98 00 00 00    lea    0x98(%rdx),%rax
 3a2:   48 8d bc ca a0 00 00    lea    0xa0(%rdx,%rcx,8),%rdi
 3a9:   00 
 3aa:   eb 09                   jmp    3b5 <usb_ifnum_to_if+0x35>
 3ac:   48 83 c0 08             add    $0x8,%rax
 3b0:   48 39 f8                cmp    %rdi,%rax
 3b3:   74 12                   je     3c7 <usb_ifnum_to_if+0x47>
 3b5:   48 8b 10                mov    (%rax),%rdx
 3b8:   48 8b 0a                mov    (%rdx),%rcx
 3bb:   0f b6 49 02             movzbl 0x2(%rcx),%ecx
 3bf:   39 f1                   cmp    %esi,%ecx
 3c1:   75 e9                   jne    3ac <usb_ifnum_to_if+0x2c>
 3c3:   48 89 d0                mov    %rdx,%rax
 3c6:   c3                      ret
 3c7:   31 d2                   xor    %edx,%edx
 3c9:   48 89 d0                mov    %rdx,%rax
 3cc:   c3                      ret
 3cd:   0f 1f 00                nopl   (%rax)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: NULL dereference on disconnection during usb_set_interface()
  2024-01-21 17:18 NULL dereference on disconnection during usb_set_interface() Michał Pecio
@ 2024-02-17 15:31 ` Greg Kroah-Hartman
  2024-02-17 19:26   ` Michał Pecio
  0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2024-02-17 15:31 UTC (permalink / raw)
  To: Michał Pecio; +Cc: linux-usb

On Sun, Jan 21, 2024 at 06:18:15PM +0100, Michał Pecio wrote:
> Hi,
> 
> I encountered an interesting race. This USB camera appears to have got
> stuck and dropped by the bus during video stream initialization, leading 
> to an oops. This is a one time occurrence, not reproducible. Maybe not
> a very severe issue due to narrow window of opportunity, but still...
> 
> Linux v6.7, the host is XHCI.
> 
> The first message below comes from uvc_video_start_transfer(). It is
> meant to be followed immediately by a call to usb_set_interface() and
> apparently during this call things went weird.
> 
> After a half second delay the device was disconnected and another five
> seconds later a NULL pointer dereference occured.
> 
> The crashing function is usb_ifnum_to_if() and disassembly suggests that
> the dereferenced NULL value was config->interface[i], for unknown i.

There are a number of known-race-conditions in the v4l interface that
can happen when devices go away and userspace is still holding a
reference on the character device node. The developers there are working
on it, but I don't know of any recent changes to help resolve this,
sorry.

Try asking on the linux-media mailing list?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: NULL dereference on disconnection during usb_set_interface()
  2024-02-17 15:31 ` Greg Kroah-Hartman
@ 2024-02-17 19:26   ` Michał Pecio
  2024-02-17 19:55     ` Alan Stern
  0 siblings, 1 reply; 5+ messages in thread
From: Michał Pecio @ 2024-02-17 19:26 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-usb

Hi Greg,

> There are a number of known-race-conditions in the v4l interface that
> can happen when devices go away and userspace is still holding a
> reference on the character device node.

I wrote to linux-usb because I think this particular crash is a bug in
the USB subsystem - namely, usb_set_interface() appears to crash when
the device is disconnected during its execution.

Indeed, today I came up with an artificial way to reproduce this crash.
I added msleep(1000) right before the call to usb_hcd_alloc_bandwidth()
in usb_set_interface() and pulled the USB plug when it slept.

(BTW, previously the device was not physically disconnected, it looks
like the host controller dropped it due to I/O errors).

Anyway, here's my new crash log:

# this is what normal execution looks like, nothing special happens yet
[  210.644611] usb_set_interface called from uvc_video_start_transfer
[  210.644615] sleeping before usb_hcd_alloc_bandwidth
[  211.668754] usb_set_interface returned

# and now I will disconnect the device during the sleep
[  216.700611] usb_set_interface called from uvc_video_start_transfer
[  216.700616] sleeping before usb_hcd_alloc_bandwidth
[  217.144340] usb 12-1.3: USB disconnect, device number 3
[  217.746182] BUG: kernel NULL pointer dereference, address: 0000000000000000
[  217.746190] #PF: supervisor read access in kernel mode
[  217.746192] #PF: error_code(0x0000) - not-present page
[  217.746195] PGD 0 P4D 0 
[  217.746197] Oops: 0000 [#1] PREEMPT SMP
[  217.746200] CPU: 0 PID: 815 Comm: yavta Not tainted 6.7.0 #4
[  217.746204] Hardware name: System manufacturer System Product Name/M4A88TD-M EVO, BIOS 1801    08/09/2012
[  217.746206] RIP: 0010:usb_ifnum_to_if+0x38/0x50
[  217.746212] Code: d2 74 32 0f b6 4a 04 84 c9 74 2e ff c9 48 8d 82 98 00 00 00 48 8d bc ca a0 00 00 00 eb 09 48 83 c0 08 48 39 f8 74 12 48 8b 10 <48> 8b 0a 0f b6 49 02 39 f1 75 e9 48 89 d0 c3 31 d2 48 89 d0 c3 0f
[  217.746215] RSP: 0018:ffffc90000b07b90 EFLAGS: 00010206
[  217.746217] RAX: ffff8880031ac498 RBX: ffff888003144800 RCX: 0000000000000003
[  217.746219] RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffff8880031ac4b8
[  217.746221] RBP: 0000000000000000 R08: 0000000000000400 R09: 0000000000000000
[  217.746223] R10: 0000000000000000 R11: 00000000000003ad R12: ffff8880031acde8
[  217.746224] R13: 0000000000000000 R14: ffff8880031acc08 R15: ffff888102ca4000
[  217.746226] FS:  00007f8455cf2740(0000) GS:ffff88811bc00000(0000) knlGS:0000000000000000
[  217.746228] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  217.746230] CR2: 0000000000000000 CR3: 000000011af26000 CR4: 00000000000006f0
[  217.746231] Call Trace:
[  217.746234]  <TASK>
[  217.746237]  ? __die+0x2d/0x80
[  217.746240]  ? page_fault_oops+0x15d/0x420
[  217.746244]  ? fixup_exception+0x36/0x280
[  217.746248]  ? exc_page_fault+0x74/0x150
[  217.746252]  ? asm_exc_page_fault+0x22/0x30
[  217.746256]  ? usb_ifnum_to_if+0x38/0x50
[  217.746258]  usb_hcd_alloc_bandwidth+0x208/0x310
[  217.746263]  ? trace_raw_output_tick_stop+0x80/0x80
[  217.746267]  usb_set_interface+0x112/0x430
[  217.746269]  ? _printk+0x48/0x50
[  217.746273]  uvc_video_start_transfer+0x1db/0x650 [uvcvideo]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: NULL dereference on disconnection during usb_set_interface()
  2024-02-17 19:26   ` Michał Pecio
@ 2024-02-17 19:55     ` Alan Stern
  2024-02-18  0:02       ` Michał Pecio
  0 siblings, 1 reply; 5+ messages in thread
From: Alan Stern @ 2024-02-17 19:55 UTC (permalink / raw)
  To: Michał Pecio; +Cc: Greg Kroah-Hartman, linux-usb

On Sat, Feb 17, 2024 at 08:26:11PM +0100, Michał Pecio wrote:
> Hi Greg,
> 
> > There are a number of known-race-conditions in the v4l interface that
> > can happen when devices go away and userspace is still holding a
> > reference on the character device node.
> 
> I wrote to linux-usb because I think this particular crash is a bug in
> the USB subsystem - namely, usb_set_interface() appears to crash when
> the device is disconnected during its execution.

No, the bug is not in the USB subsystem.  Drivers are not supposed to 
call usb_set_interface() unless they can guarantee that the device will 
not be removed while the call is in progress, generally by holding the 
device lock.

I suppose it could be considered a bug that this requirement is not 
documented at the start of the function.  You could submit a patch 
adding an appropriate section to the kerneldoc.

> Indeed, today I came up with an artificial way to reproduce this crash.
> I added msleep(1000) right before the call to usb_hcd_alloc_bandwidth()
> in usb_set_interface() and pulled the USB plug when it slept.
> 
> (BTW, previously the device was not physically disconnected, it looks
> like the host controller dropped it due to I/O errors).
> 
> Anyway, here's my new crash log:
> 
> # this is what normal execution looks like, nothing special happens yet
> [  210.644611] usb_set_interface called from uvc_video_start_transfer

Here you see usb_set_interface() called from the UVC driver under 
inappropriate circumstances.  To fix the problem it will be necessary 
to fix the UVC driver.  It should not allow itself to be unbound from 
the device while a user-driven operation, such as 
uvc_video_start_transfer(), is in progress.

Alan Stern

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: NULL dereference on disconnection during usb_set_interface()
  2024-02-17 19:55     ` Alan Stern
@ 2024-02-18  0:02       ` Michał Pecio
  0 siblings, 0 replies; 5+ messages in thread
From: Michał Pecio @ 2024-02-18  0:02 UTC (permalink / raw)
  To: Alan Stern; +Cc: Greg Kroah-Hartman, linux-usb

> Here you see usb_set_interface() called from the UVC driver under 
> inappropriate circumstances.  To fix the problem it will be necessary 
> to fix the UVC driver.  It should not allow itself to be unbound from 
> the device while a user-driven operation, such as 
> uvc_video_start_transfer(), is in progress.

Thanks for your explanation, I can see that this is indeed what happens.

I produced a small documentation update patch, hopefully I got it right
this time.

I will notify linux-media about the issue.

Thanks,
Michal

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-02-18  0:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-21 17:18 NULL dereference on disconnection during usb_set_interface() Michał Pecio
2024-02-17 15:31 ` Greg Kroah-Hartman
2024-02-17 19:26   ` Michał Pecio
2024-02-17 19:55     ` Alan Stern
2024-02-18  0:02       ` Michał Pecio

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).