[PATCH v3] usb/cdc-wdm: fix memory info leak in wdm_read
From: Sabyrzhan Tasbolatov
Date: Mon Nov 11 2024 - 07:01:59 EST
syzbot reported "KMSAN: kernel-infoleak in wdm_read", though there is no
reproducer and the only report for this issue.
The check:
if (cntr > count)
cntr = count;
only limits `cntr` to `count` (the number of bytes requested by
userspace), but it doesn't verify that `desc->ubuf` actually has `count`
bytes. This oversight can lead to situations where `copy_to_user` reads
uninitialized data from `desc->ubuf`.
This patch makes sure `cntr` respects` both the `desc->length` and the
`count` requested by userspace, preventing any uninitialized memory from
leaking into userspace.
syzbot report
=============
BUG: KMSAN: kernel-infoleak in instrument_copy_to_user include/linux/instrumented.h:114 [inline]
BUG: KMSAN: kernel-infoleak in _inline_copy_to_user include/linux/uaccess.h:180 [inline]
BUG: KMSAN: kernel-infoleak in _copy_to_user+0xbc/0x110 lib/usercopy.c:26
instrument_copy_to_user include/linux/instrumented.h:114 [inline]
_inline_copy_to_user include/linux/uaccess.h:180 [inline]
_copy_to_user+0xbc/0x110 lib/usercopy.c:26
copy_to_user include/linux/uaccess.h:209 [inline]
wdm_read+0x227/0x1270 drivers/usb/class/cdc-wdm.c:603
vfs_read+0x2a1/0xf60 fs/read_write.c:474
ksys_read+0x20f/0x4c0 fs/read_write.c:619
__do_sys_read fs/read_write.c:629 [inline]
__se_sys_read fs/read_write.c:627 [inline]
__x64_sys_read+0x93/0xe0 fs/read_write.c:627
x64_sys_call+0x3055/0x3ba0 arch/x86/include/generated/asm/syscalls_64.h:1
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xcd/0x1e0 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Reported-by: syzbot+9760fbbd535cee131f81@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=9760fbbd535cee131f81
Signed-off-by: Sabyrzhan Tasbolatov <snovitoll@xxxxxxxxx>
---
Changes v2 -> v3:
- reverted kzalloc back to kmalloc as the fix is cntr related (Oliver).
- added constraint to select the min length from count and desc->length.
- refactored git commit description as the memory info leak is confirmed.
Changes v1 -> v2:
- added explanation comment above kzalloc (Greg).
- renamed patch title from memory leak to memory info leak (Greg).
---
drivers/usb/class/cdc-wdm.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c
index 86ee39db013f..dd7349f8a97a 100644
--- a/drivers/usb/class/cdc-wdm.c
+++ b/drivers/usb/class/cdc-wdm.c
@@ -598,8 +598,9 @@ static ssize_t wdm_read
spin_unlock_irq(&desc->iuspin);
}
- if (cntr > count)
- cntr = count;
+ /* Ensure cntr does not exceed available data in ubuf. */
+ cntr = min(count, (size_t) desc->length);
+
rv = copy_to_user(buffer, desc->ubuf, cntr);
if (rv > 0) {
rv = -EFAULT;
--
2.34.1