All of lore.kernel.org
 help / color / mirror / Atom feed
* [syzbot] [usb?] [media?] possible deadlock in vb2_video_unregister_device
@ 2024-02-07  8:44 syzbot
  2024-02-07 11:07 ` Hillf Danton
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: syzbot @ 2024-02-07  8:44 UTC (permalink / raw)
  To: linux-kernel, linux-media, linux-usb, m.szyprowski, mchehab,
	syzkaller-bugs, tfiga

Hello,

syzbot found the following issue on:

HEAD commit:    ed5551279c91 Merge 6.8-rc3 into usb-next
git tree:       https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
console output: https://syzkaller.appspot.com/x/log.txt?x=16940d7be80000
kernel config:  https://syzkaller.appspot.com/x/.config?x=28a3704ea90ef255
dashboard link: https://syzkaller.appspot.com/bug?extid=3b1d4b3d5f7a358bf9a9
compiler:       gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14450a38180000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1603629fe80000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/10b543a7171a/disk-ed555127.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/dc10f27643e8/vmlinux-ed555127.xz
kernel image: https://storage.googleapis.com/syzbot-assets/bb6389e754b9/bzImage-ed555127.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+3b1d4b3d5f7a358bf9a9@syzkaller.appspotmail.com

usb 1-1: SerialNumber: syz
usb 1-1: config 0 descriptor??
usbtv 1-1:0.0: Fushicai USBTV007 Audio-Video Grabber
usb 1-1: USB disconnect, device number 2
============================================
WARNING: possible recursive locking detected
6.8.0-rc3-syzkaller-00047-ged5551279c91 #0 Not tainted
--------------------------------------------
kworker/0:2/694 is trying to acquire lock:
ffff888109f20b70 (&usbtv->vb2q_lock){+.+.}-{3:3}, at: vb2_video_unregister_device drivers/media/common/videobuf2/videobuf2-v4l2.c:1269 [inline]
ffff888109f20b70 (&usbtv->vb2q_lock){+.+.}-{3:3}, at: vb2_video_unregister_device+0x12b/0x2d0 drivers/media/common/videobuf2/videobuf2-v4l2.c:1245

but task is already holding lock:
ffff888109f20b70 (&usbtv->vb2q_lock){+.+.}-{3:3}, at: usbtv_video_free+0x28/0x70 drivers/media/usb/usbtv/usbtv-video.c:966

other info that might help us debug this:
 Possible unsafe locking scenario:

       CPU0
       ----
  lock(&usbtv->vb2q_lock);
  lock(&usbtv->vb2q_lock);

 *** DEADLOCK ***

 May be due to missing lock nesting notation

7 locks held by kworker/0:2/694:
 #0: ffff888106ad2138 ((wq_completion)usb_hub_wq){+.+.}-{0:0}, at: process_one_work+0x789/0x15d0 kernel/workqueue.c:2608
 #1: ffffc90001c0fd80 ((work_completion)(&hub->events)){+.+.}-{0:0}, at: process_one_work+0x7eb/0x15d0 kernel/workqueue.c:2609
 #2: ffff88810af2f190 (&dev->mutex){....}-{3:3}, at: device_lock include/linux/device.h:990 [inline]
 #2: ffff88810af2f190 (&dev->mutex){....}-{3:3}, at: hub_event+0x1be/0x4f40 drivers/usb/core/hub.c:5840
 #3: ffff888107799190 (&dev->mutex){....}-{3:3}, at: device_lock include/linux/device.h:990 [inline]
 #3: ffff888107799190 (&dev->mutex){....}-{3:3}, at: usb_disconnect+0x10a/0x910 drivers/usb/core/hub.c:2287
 #4: ffff88810779a160 (&dev->mutex){....}-{3:3}, at: device_lock include/linux/device.h:990 [inline]
 #4: ffff88810779a160 (&dev->mutex){....}-{3:3}, at: __device_driver_lock drivers/base/dd.c:1095 [inline]
 #4: ffff88810779a160 (&dev->mutex){....}-{3:3}, at: device_release_driver_internal+0xa4/0x610 drivers/base/dd.c:1292
 #5: ffff888109f20b70 (&usbtv->vb2q_lock){+.+.}-{3:3}, at: usbtv_video_free+0x28/0x70 drivers/media/usb/usbtv/usbtv-video.c:966
 #6: ffff888109f20ae0 (&usbtv->v4l2_lock){+.+.}-{3:3}, at: usbtv_video_free+0x32/0x70 drivers/media/usb/usbtv/usbtv-video.c:967

stack backtrace:
CPU: 0 PID: 694 Comm: kworker/0:2 Not tainted 6.8.0-rc3-syzkaller-00047-ged5551279c91 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/25/2024
Workqueue: usb_hub_wq hub_event
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0xd9/0x1b0 lib/dump_stack.c:106
 check_deadlock kernel/locking/lockdep.c:3062 [inline]
 validate_chain kernel/locking/lockdep.c:3856 [inline]
 __lock_acquire+0x210a/0x3b30 kernel/locking/lockdep.c:5137
 lock_acquire kernel/locking/lockdep.c:5754 [inline]
 lock_acquire+0x1ae/0x520 kernel/locking/lockdep.c:5719
 __mutex_lock_common kernel/locking/mutex.c:608 [inline]
 __mutex_lock+0x175/0x9d0 kernel/locking/mutex.c:752
 vb2_video_unregister_device drivers/media/common/videobuf2/videobuf2-v4l2.c:1269 [inline]
 vb2_video_unregister_device+0x12b/0x2d0 drivers/media/common/videobuf2/videobuf2-v4l2.c:1245
 usbtv_video_free+0x4a/0x70 drivers/media/usb/usbtv/usbtv-video.c:970
 usbtv_disconnect+0x5c/0xd0 drivers/media/usb/usbtv/usbtv-core.c:138
 usb_unbind_interface+0x1e5/0x960 drivers/usb/core/driver.c:461
 device_remove drivers/base/dd.c:569 [inline]
 device_remove+0x11f/0x170 drivers/base/dd.c:561
 __device_release_driver drivers/base/dd.c:1272 [inline]
 device_release_driver_internal+0x44a/0x610 drivers/base/dd.c:1295
 bus_remove_device+0x22c/0x420 drivers/base/bus.c:574
 device_del+0x39a/0xa50 drivers/base/core.c:3814
 usb_disable_device+0x36c/0x7f0 drivers/usb/core/message.c:1416
 usb_disconnect+0x2e1/0x910 drivers/usb/core/hub.c:2296
 hub_port_connect drivers/usb/core/hub.c:5352 [inline]
 hub_port_connect_change drivers/usb/core/hub.c:5652 [inline]
 port_event drivers/usb/core/hub.c:5812 [inline]
 hub_event+0x1be0/0x4f40 drivers/usb/core/hub.c:5894
 process_one_work+0x886/0x15d0 kernel/workqueue.c:2633
 process_scheduled_works kernel/workqueue.c:2706 [inline]
 worker_thread+0x8b9/0x1290 kernel/workqueue.c:2787
 kthread+0x2c6/0x3a0 kernel/kthread.c:388
 ret_from_fork+0x45/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:242
 </TASK>


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

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

* Re: [syzbot] [usb?] [media?] possible deadlock in vb2_video_unregister_device
  2024-02-07  8:44 [syzbot] [usb?] [media?] possible deadlock in vb2_video_unregister_device syzbot
@ 2024-02-07 11:07 ` Hillf Danton
  2024-02-07 11:24   ` syzbot
  2024-02-16  5:45 ` Tomasz Figa
  2024-02-19 13:10 ` syzbot
  2 siblings, 1 reply; 7+ messages in thread
From: Hillf Danton @ 2024-02-07 11:07 UTC (permalink / raw)
  To: syzbot; +Cc: linux-kernel, syzkaller-bugs

On Wed, 07 Feb 2024 00:44:22 -0800
> HEAD commit:    ed5551279c91 Merge 6.8-rc3 into usb-next
> git tree:       https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1603629fe80000

#syz test https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git  usb-testing

--- x/drivers/media/usb/usbtv/usbtv-video.c
+++ y/drivers/media/usb/usbtv/usbtv-video.c
@@ -963,11 +963,11 @@ ctrl_fail:
 
 void usbtv_video_free(struct usbtv *usbtv)
 {
+	usbtv_stop(usbtv);
+	vb2_video_unregister_device(&usbtv->vdev);
 	mutex_lock(&usbtv->vb2q_lock);
 	mutex_lock(&usbtv->v4l2_lock);
 
-	usbtv_stop(usbtv);
-	vb2_video_unregister_device(&usbtv->vdev);
 	v4l2_device_disconnect(&usbtv->v4l2_dev);
 
 	mutex_unlock(&usbtv->v4l2_lock);
--

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

* Re: [syzbot] [usb?] [media?] possible deadlock in vb2_video_unregister_device
  2024-02-07 11:07 ` Hillf Danton
@ 2024-02-07 11:24   ` syzbot
  0 siblings, 0 replies; 7+ messages in thread
From: syzbot @ 2024-02-07 11:24 UTC (permalink / raw)
  To: hdanton, linux-kernel, syzkaller-bugs

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+3b1d4b3d5f7a358bf9a9@syzkaller.appspotmail.com

Tested on:

commit:         ed555127 Merge 6.8-rc3 into usb-next
git tree:       https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
console output: https://syzkaller.appspot.com/x/log.txt?x=13796624180000
kernel config:  https://syzkaller.appspot.com/x/.config?x=28a3704ea90ef255
dashboard link: https://syzkaller.appspot.com/bug?extid=3b1d4b3d5f7a358bf9a9
compiler:       gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
patch:          https://syzkaller.appspot.com/x/patch.diff?x=15126718180000

Note: testing is done by a robot and is best-effort only.

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

* Re: [syzbot] [usb?] [media?] possible deadlock in vb2_video_unregister_device
  2024-02-07  8:44 [syzbot] [usb?] [media?] possible deadlock in vb2_video_unregister_device syzbot
  2024-02-07 11:07 ` Hillf Danton
@ 2024-02-16  5:45 ` Tomasz Figa
  2024-02-19 13:10 ` syzbot
  2 siblings, 0 replies; 7+ messages in thread
From: Tomasz Figa @ 2024-02-16  5:45 UTC (permalink / raw)
  To: syzbot, Hans Verkuil, Laurent Pinchart, mchehab
  Cc: linux-kernel, linux-media, linux-usb, m.szyprowski, syzkaller-bugs

On Wed, Feb 7, 2024 at 5:44 PM syzbot
<syzbot+3b1d4b3d5f7a358bf9a9@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit:    ed5551279c91 Merge 6.8-rc3 into usb-next
> git tree:       https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
> console output: https://syzkaller.appspot.com/x/log.txt?x=16940d7be80000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=28a3704ea90ef255
> dashboard link: https://syzkaller.appspot.com/bug?extid=3b1d4b3d5f7a358bf9a9
> compiler:       gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14450a38180000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1603629fe80000
>
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/10b543a7171a/disk-ed555127.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/dc10f27643e8/vmlinux-ed555127.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/bb6389e754b9/bzImage-ed555127.xz
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+3b1d4b3d5f7a358bf9a9@syzkaller.appspotmail.com
>
> usb 1-1: SerialNumber: syz
> usb 1-1: config 0 descriptor??
> usbtv 1-1:0.0: Fushicai USBTV007 Audio-Video Grabber
> usb 1-1: USB disconnect, device number 2
> ============================================
> WARNING: possible recursive locking detected
> 6.8.0-rc3-syzkaller-00047-ged5551279c91 #0 Not tainted
> --------------------------------------------
> kworker/0:2/694 is trying to acquire lock:
> ffff888109f20b70 (&usbtv->vb2q_lock){+.+.}-{3:3}, at: vb2_video_unregister_device drivers/media/common/videobuf2/videobuf2-v4l2.c:1269 [inline]
> ffff888109f20b70 (&usbtv->vb2q_lock){+.+.}-{3:3}, at: vb2_video_unregister_device+0x12b/0x2d0 drivers/media/common/videobuf2/videobuf2-v4l2.c:1245
>
> but task is already holding lock:
> ffff888109f20b70 (&usbtv->vb2q_lock){+.+.}-{3:3}, at: usbtv_video_free+0x28/0x70 drivers/media/usb/usbtv/usbtv-video.c:966
>

Looking at usbtv_video_free [1], it calls
vb2_video_unregister_device() with the queue mutex locked, which is
just wrong. I wonder how (and if) it even worked before. Laurent,
Hans, Mauro, any idea who is maintaining this driver? I don't see a
matching entry in the MAINTAINERS file.

[1] https://elixir.bootlin.com/linux/v6.8-rc4/source/drivers/media/usb/usbtv/usbtv-video.c#L964

Best regards,
Tomasz

> other info that might help us debug this:
>  Possible unsafe locking scenario:
>
>        CPU0
>        ----
>   lock(&usbtv->vb2q_lock);
>   lock(&usbtv->vb2q_lock);
>
>  *** DEADLOCK ***
>
>  May be due to missing lock nesting notation
>
> 7 locks held by kworker/0:2/694:
>  #0: ffff888106ad2138 ((wq_completion)usb_hub_wq){+.+.}-{0:0}, at: process_one_work+0x789/0x15d0 kernel/workqueue.c:2608
>  #1: ffffc90001c0fd80 ((work_completion)(&hub->events)){+.+.}-{0:0}, at: process_one_work+0x7eb/0x15d0 kernel/workqueue.c:2609
>  #2: ffff88810af2f190 (&dev->mutex){....}-{3:3}, at: device_lock include/linux/device.h:990 [inline]
>  #2: ffff88810af2f190 (&dev->mutex){....}-{3:3}, at: hub_event+0x1be/0x4f40 drivers/usb/core/hub.c:5840
>  #3: ffff888107799190 (&dev->mutex){....}-{3:3}, at: device_lock include/linux/device.h:990 [inline]
>  #3: ffff888107799190 (&dev->mutex){....}-{3:3}, at: usb_disconnect+0x10a/0x910 drivers/usb/core/hub.c:2287
>  #4: ffff88810779a160 (&dev->mutex){....}-{3:3}, at: device_lock include/linux/device.h:990 [inline]
>  #4: ffff88810779a160 (&dev->mutex){....}-{3:3}, at: __device_driver_lock drivers/base/dd.c:1095 [inline]
>  #4: ffff88810779a160 (&dev->mutex){....}-{3:3}, at: device_release_driver_internal+0xa4/0x610 drivers/base/dd.c:1292
>  #5: ffff888109f20b70 (&usbtv->vb2q_lock){+.+.}-{3:3}, at: usbtv_video_free+0x28/0x70 drivers/media/usb/usbtv/usbtv-video.c:966
>  #6: ffff888109f20ae0 (&usbtv->v4l2_lock){+.+.}-{3:3}, at: usbtv_video_free+0x32/0x70 drivers/media/usb/usbtv/usbtv-video.c:967
>
> stack backtrace:
> CPU: 0 PID: 694 Comm: kworker/0:2 Not tainted 6.8.0-rc3-syzkaller-00047-ged5551279c91 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/25/2024
> Workqueue: usb_hub_wq hub_event
> Call Trace:
>  <TASK>
>  __dump_stack lib/dump_stack.c:88 [inline]
>  dump_stack_lvl+0xd9/0x1b0 lib/dump_stack.c:106
>  check_deadlock kernel/locking/lockdep.c:3062 [inline]
>  validate_chain kernel/locking/lockdep.c:3856 [inline]
>  __lock_acquire+0x210a/0x3b30 kernel/locking/lockdep.c:5137
>  lock_acquire kernel/locking/lockdep.c:5754 [inline]
>  lock_acquire+0x1ae/0x520 kernel/locking/lockdep.c:5719
>  __mutex_lock_common kernel/locking/mutex.c:608 [inline]
>  __mutex_lock+0x175/0x9d0 kernel/locking/mutex.c:752
>  vb2_video_unregister_device drivers/media/common/videobuf2/videobuf2-v4l2.c:1269 [inline]
>  vb2_video_unregister_device+0x12b/0x2d0 drivers/media/common/videobuf2/videobuf2-v4l2.c:1245
>  usbtv_video_free+0x4a/0x70 drivers/media/usb/usbtv/usbtv-video.c:970
>  usbtv_disconnect+0x5c/0xd0 drivers/media/usb/usbtv/usbtv-core.c:138
>  usb_unbind_interface+0x1e5/0x960 drivers/usb/core/driver.c:461
>  device_remove drivers/base/dd.c:569 [inline]
>  device_remove+0x11f/0x170 drivers/base/dd.c:561
>  __device_release_driver drivers/base/dd.c:1272 [inline]
>  device_release_driver_internal+0x44a/0x610 drivers/base/dd.c:1295
>  bus_remove_device+0x22c/0x420 drivers/base/bus.c:574
>  device_del+0x39a/0xa50 drivers/base/core.c:3814
>  usb_disable_device+0x36c/0x7f0 drivers/usb/core/message.c:1416
>  usb_disconnect+0x2e1/0x910 drivers/usb/core/hub.c:2296
>  hub_port_connect drivers/usb/core/hub.c:5352 [inline]
>  hub_port_connect_change drivers/usb/core/hub.c:5652 [inline]
>  port_event drivers/usb/core/hub.c:5812 [inline]
>  hub_event+0x1be0/0x4f40 drivers/usb/core/hub.c:5894
>  process_one_work+0x886/0x15d0 kernel/workqueue.c:2633
>  process_scheduled_works kernel/workqueue.c:2706 [inline]
>  worker_thread+0x8b9/0x1290 kernel/workqueue.c:2787
>  kthread+0x2c6/0x3a0 kernel/kthread.c:388
>  ret_from_fork+0x45/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:242
>  </TASK>
>
>
> ---
> This report is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this issue. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
>
> If the report is already addressed, let syzbot know by replying with:
> #syz fix: exact-commit-title
>
> If you want syzbot to run the reproducer, reply with:
> #syz test: git://repo/address.git branch-or-commit-hash
> If you attach or paste a git patch, syzbot will apply it before testing.
>
> If you want to overwrite report's subsystems, reply with:
> #syz set subsystems: new-subsystem
> (See the list of subsystem names on the web dashboard)
>
> If the report is a duplicate of another one, reply with:
> #syz dup: exact-subject-of-another-report
>
> If you want to undo deduplication, reply with:
> #syz undup

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

* Re: [syzbot] [usb?] [media?] possible deadlock in vb2_video_unregister_device
  2024-02-07  8:44 [syzbot] [usb?] [media?] possible deadlock in vb2_video_unregister_device syzbot
  2024-02-07 11:07 ` Hillf Danton
  2024-02-16  5:45 ` Tomasz Figa
@ 2024-02-19 13:10 ` syzbot
  2024-02-19 15:49   ` Benjamin Gaignard
  2 siblings, 1 reply; 7+ messages in thread
From: syzbot @ 2024-02-19 13:10 UTC (permalink / raw)
  To: benjamin.gaignard, hdanton, hverkuil-cisco, hverkuil,
	laurent.pinchart, linux-kernel, linux-media, linux-usb,
	m.szyprowski, mchehab, syzkaller-bugs, tfiga

syzbot has bisected this issue to:

commit c838530d230bc638d79b78737fc4488ffc28c1ee
Author: Benjamin Gaignard <benjamin.gaignard@collabora.com>
Date:   Thu Nov 9 16:34:59 2023 +0000

    media: media videobuf2: Be more flexible on the number of queue stored buffers

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=166dc872180000
start commit:   b401b621758e Linux 6.8-rc5
git tree:       upstream
final oops:     https://syzkaller.appspot.com/x/report.txt?x=156dc872180000
console output: https://syzkaller.appspot.com/x/log.txt?x=116dc872180000
kernel config:  https://syzkaller.appspot.com/x/.config?x=eff9f3183d0a20dd
dashboard link: https://syzkaller.appspot.com/bug?extid=3b1d4b3d5f7a358bf9a9
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=13ffaae8180000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=13ef909c180000

Reported-by: syzbot+3b1d4b3d5f7a358bf9a9@syzkaller.appspotmail.com
Fixes: c838530d230b ("media: media videobuf2: Be more flexible on the number of queue stored buffers")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection

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

* Re: [syzbot] [usb?] [media?] possible deadlock in vb2_video_unregister_device
  2024-02-19 13:10 ` syzbot
@ 2024-02-19 15:49   ` Benjamin Gaignard
  2024-02-20  4:49     ` Tomasz Figa
  0 siblings, 1 reply; 7+ messages in thread
From: Benjamin Gaignard @ 2024-02-19 15:49 UTC (permalink / raw)
  To: syzbot, hdanton, hverkuil-cisco, hverkuil, laurent.pinchart,
	linux-kernel, linux-media, linux-usb, m.szyprowski, mchehab,
	syzkaller-bugs, tfiga


Le 19/02/2024 à 14:10, syzbot a écrit :
> syzbot has bisected this issue to:
>
> commit c838530d230bc638d79b78737fc4488ffc28c1ee
> Author: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> Date:   Thu Nov 9 16:34:59 2023 +0000
>
>      media: media videobuf2: Be more flexible on the number of queue stored buffers
>
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=166dc872180000
> start commit:   b401b621758e Linux 6.8-rc5
> git tree:       upstream
> final oops:     https://syzkaller.appspot.com/x/report.txt?x=156dc872180000
> console output: https://syzkaller.appspot.com/x/log.txt?x=116dc872180000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=eff9f3183d0a20dd
> dashboard link: https://syzkaller.appspot.com/bug?extid=3b1d4b3d5f7a358bf9a9
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=13ffaae8180000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=13ef909c180000
>
> Reported-by: syzbot+3b1d4b3d5f7a358bf9a9@syzkaller.appspotmail.com
> Fixes: c838530d230b ("media: media videobuf2: Be more flexible on the number of queue stored buffers")
>
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection

Hans,
I think the issue occur because of this part of the commit:
@@ -1264,7 +1264,7 @@ void vb2_video_unregister_device(struct video_device *vdev)
          */
         get_device(&vdev->dev);
         video_unregister_device(vdev);
-       if (vdev->queue && vdev->queue->owner) {
+       if (vdev->queue) {
                 struct mutex *lock = vdev->queue->lock ?
                         vdev->queue->lock : vdev->lock;

but I wonder if the correction shouldn't be to remove usbtv->vb2q_lock mutex in usbtv_video_free().

Any opinion ?

Regards,
Benjamin

>

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

* Re: [syzbot] [usb?] [media?] possible deadlock in vb2_video_unregister_device
  2024-02-19 15:49   ` Benjamin Gaignard
@ 2024-02-20  4:49     ` Tomasz Figa
  0 siblings, 0 replies; 7+ messages in thread
From: Tomasz Figa @ 2024-02-20  4:49 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: syzbot, hdanton, hverkuil-cisco, hverkuil, laurent.pinchart,
	linux-kernel, linux-media, linux-usb, m.szyprowski, mchehab,
	syzkaller-bugs

On Tue, Feb 20, 2024 at 12:49 AM Benjamin Gaignard
<benjamin.gaignard@collabora.com> wrote:
>
>
> Le 19/02/2024 à 14:10, syzbot a écrit :
> > syzbot has bisected this issue to:
> >
> > commit c838530d230bc638d79b78737fc4488ffc28c1ee
> > Author: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> > Date:   Thu Nov 9 16:34:59 2023 +0000
> >
> >      media: media videobuf2: Be more flexible on the number of queue stored buffers
> >
> > bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=166dc872180000
> > start commit:   b401b621758e Linux 6.8-rc5
> > git tree:       upstream
> > final oops:     https://syzkaller.appspot.com/x/report.txt?x=156dc872180000
> > console output: https://syzkaller.appspot.com/x/log.txt?x=116dc872180000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=eff9f3183d0a20dd
> > dashboard link: https://syzkaller.appspot.com/bug?extid=3b1d4b3d5f7a358bf9a9
> > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=13ffaae8180000
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=13ef909c180000
> >
> > Reported-by: syzbot+3b1d4b3d5f7a358bf9a9@syzkaller.appspotmail.com
> > Fixes: c838530d230b ("media: media videobuf2: Be more flexible on the number of queue stored buffers")
> >
> > For information about bisection process see: https://goo.gl/tpsmEJ#bisection
>
> Hans,
> I think the issue occur because of this part of the commit:
> @@ -1264,7 +1264,7 @@ void vb2_video_unregister_device(struct video_device *vdev)
>           */
>          get_device(&vdev->dev);
>          video_unregister_device(vdev);
> -       if (vdev->queue && vdev->queue->owner) {
> +       if (vdev->queue) {
>                  struct mutex *lock = vdev->queue->lock ?
>                          vdev->queue->lock : vdev->lock;
>
> but I wonder if the correction shouldn't be to remove usbtv->vb2q_lock mutex in usbtv_video_free().
>
> Any opinion ?

That is probably what triggered the failure detected by syzbot, but I
don't think we've ever had a guarantee that owner is NULL in that
code.

Looking at the uvc driver [1], it doesn't seem to need any special
locking there (after all the vb2 code acquires them).  (It also
doesn't have the custom clean-up code that the usbtv driver has in
usbtv_stop(), but that's another story). So maybe all we need to fix
it is removing the mutex_lock/unlock() calls?

[1] https://elixir.bootlin.com/linux/v6.8-rc4/source/drivers/media/usb/uvc/uvc_driver.c#L1906

Best,
Tomasz

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

end of thread, other threads:[~2024-02-20  4:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-07  8:44 [syzbot] [usb?] [media?] possible deadlock in vb2_video_unregister_device syzbot
2024-02-07 11:07 ` Hillf Danton
2024-02-07 11:24   ` syzbot
2024-02-16  5:45 ` Tomasz Figa
2024-02-19 13:10 ` syzbot
2024-02-19 15:49   ` Benjamin Gaignard
2024-02-20  4:49     ` Tomasz Figa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.