Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

media: rc: fix races with imon_disconnect()

Syzbot reports a KASAN issue as below:
BUG: KASAN: use-after-free in __create_pipe include/linux/usb.h:1945 [inline]
BUG: KASAN: use-after-free in send_packet+0xa2d/0xbc0 drivers/media/rc/imon.c:627
Read of size 4 at addr ffff8880256fb000 by task syz-executor314/4465

CPU: 2 PID: 4465 Comm: syz-executor314 Not tainted 6.0.0-rc1-syzkaller #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
print_address_description mm/kasan/report.c:317 [inline]
print_report.cold+0x2ba/0x6e9 mm/kasan/report.c:433
kasan_report+0xb1/0x1e0 mm/kasan/report.c:495
__create_pipe include/linux/usb.h:1945 [inline]
send_packet+0xa2d/0xbc0 drivers/media/rc/imon.c:627
vfd_write+0x2d9/0x550 drivers/media/rc/imon.c:991
vfs_write+0x2d7/0xdd0 fs/read_write.c:576
ksys_write+0x127/0x250 fs/read_write.c:631
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd

The iMON driver improperly releases the usb_device reference in
imon_disconnect without coordinating with active users of the
device.

Specifically, the fields usbdev_intf0 and usbdev_intf1 are not
protected by the users counter (ictx->users). During probe,
imon_init_intf0 or imon_init_intf1 increments the usb_device
reference count depending on the interface. However, during
disconnect, usb_put_dev is called unconditionally, regardless of
actual usage.

As a result, if vfd_write or other operations are still in
progress after disconnect, this can lead to a use-after-free of
the usb_device pointer.

Thread 1 vfd_write Thread 2 imon_disconnect
...
if
usb_put_dev(ictx->usbdev_intf0)
else
usb_put_dev(ictx->usbdev_intf1)
...
while
send_packet
if
pipe = usb_sndintpipe(
ictx->usbdev_intf0) UAF
else
pipe = usb_sndctrlpipe(
ictx->usbdev_intf0, 0) UAF

Guard access to usbdev_intf0 and usbdev_intf1 after disconnect by
checking ictx->disconnected in all writer paths. Add early return
with -ENODEV in send_packet(), vfd_write(), lcd_write() and
display_open() if the device is no longer present.

Set and read ictx->disconnected under ictx->lock to ensure memory
synchronization. Acquire the lock in imon_disconnect() before setting
the flag to synchronize with any ongoing operations.

Ensure writers exit early and safely after disconnect before the USB
core proceeds with cleanup.

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.

Reported-by: syzbot+f1a69784f6efe748c3bf@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f1a69784f6efe748c3bf
Fixes: 21677cfc562a ("V4L/DVB: ir-core: add imon driver")
Cc: stable@vger.kernel.org

Signed-off-by: Larshin Sergey <Sergey.Larshin@kaspersky.com>
Signed-off-by: Sean Young <sean@mess.org>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>

authored by

Larshin Sergey and committed by
Hans Verkuil
fa0f61cc 76d2d8f7

+20 -7
+20 -7
drivers/media/rc/imon.c
··· 531 531 532 532 mutex_lock(&ictx->lock); 533 533 534 - if (!ictx->display_supported) { 534 + if (ictx->disconnected) { 535 + retval = -ENODEV; 536 + } else if (!ictx->display_supported) { 535 537 pr_err("display not supported by device\n"); 536 538 retval = -ENODEV; 537 539 } else if (ictx->display_isopen) { ··· 596 594 struct usb_ctrlrequest *control_req = NULL; 597 595 598 596 lockdep_assert_held(&ictx->lock); 597 + 598 + if (ictx->disconnected) 599 + return -ENODEV; 599 600 600 601 /* Check if we need to use control or interrupt urb */ 601 602 if (!ictx->tx_control) { ··· 954 949 static const unsigned char vfd_packet6[] = { 955 950 0x01, 0x00, 0x00, 0x00, 0x00, 0xFF, 0xFF }; 956 951 957 - if (ictx->disconnected) 958 - return -ENODEV; 959 - 960 952 if (mutex_lock_interruptible(&ictx->lock)) 961 953 return -ERESTARTSYS; 954 + 955 + if (ictx->disconnected) { 956 + retval = -ENODEV; 957 + goto exit; 958 + } 962 959 963 960 if (!ictx->dev_present_intf0) { 964 961 pr_err_ratelimited("no iMON device present\n"); ··· 1036 1029 int retval = 0; 1037 1030 struct imon_context *ictx = file->private_data; 1038 1031 1039 - if (ictx->disconnected) 1040 - return -ENODEV; 1041 - 1042 1032 mutex_lock(&ictx->lock); 1033 + 1034 + if (ictx->disconnected) { 1035 + retval = -ENODEV; 1036 + goto exit; 1037 + } 1043 1038 1044 1039 if (!ictx->display_supported) { 1045 1040 pr_err_ratelimited("no iMON display present\n"); ··· 2516 2507 int ifnum; 2517 2508 2518 2509 ictx = usb_get_intfdata(interface); 2510 + 2511 + mutex_lock(&ictx->lock); 2519 2512 ictx->disconnected = true; 2513 + mutex_unlock(&ictx->lock); 2514 + 2520 2515 dev = ictx->dev; 2521 2516 ifnum = interface->cur_altsetting->desc.bInterfaceNumber; 2522 2517