| Message ID | 20241112132931.3504749-1-snovitoll@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4] usb/cdc-wdm: fix memory info leak in wdm_read | expand |
diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index 86ee39db013f..5a500973b463 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_t(size_t, count, desc->length); + rv = copy_to_user(buffer, desc->ubuf, cntr); if (rv > 0) { rv = -EFAULT;
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@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=9760fbbd535cee131f81 Signed-off-by: Sabyrzhan Tasbolatov <snovitoll@gmail.com> --- Changes v3 -> v4: - changed min() to min_t() due to type safety (Greg). 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(-)