public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [syzbot] [sound?] [usb?] KASAN: slab-out-of-bounds Write in copy_to_urb
@ 2025-11-05 15:20 syzbot
  2025-11-06  2:10 ` Hillf Danton
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: syzbot @ 2025-11-05 15:20 UTC (permalink / raw)
  To: linux-kernel, linux-sound, linux-usb, perex, syzkaller-bugs,
	tiwai

Hello,

syzbot found the following issue on:

HEAD commit:    ba36dd5ee6fd Merge tag 'bpf-fixes' of git://git.kernel.org..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1759afe2580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=929790bc044e87d7
dashboard link: https://syzkaller.appspot.com/bug?extid=bfd77469c8966de076f7
compiler:       Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=11c74292580000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=107bffe2580000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/d900f083ada3/non_bootable_disk-ba36dd5e.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/f3d081e7b0db/vmlinux-ba36dd5e.xz
kernel image: https://storage.googleapis.com/syzbot-assets/612c239b909b/bzImage-ba36dd5e.xz

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

==================================================================
BUG: KASAN: slab-out-of-bounds in copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
Write of size 264 at addr ffff88801107b400 by task syz.0.17/5461

CPU: 0 UID: 0 PID: 5461 Comm: syz.0.17 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
Call Trace:
 <TASK>
 dump_stack_lvl+0x189/0x250 lib/dump_stack.c:120
 print_address_description mm/kasan/report.c:378 [inline]
 print_report+0xca/0x240 mm/kasan/report.c:482
 kasan_report+0x118/0x150 mm/kasan/report.c:595
 check_region_inline mm/kasan/generic.c:-1 [inline]
 kasan_check_range+0x2b0/0x2c0 mm/kasan/generic.c:200
 __asan_memcpy+0x40/0x70 mm/kasan/shadow.c:106
 copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
 prepare_playback_urb+0x953/0x13d0 sound/usb/pcm.c:1611
 prepare_outbound_urb+0x377/0xc50 sound/usb/endpoint.c:333
 snd_usb_endpoint_start+0x4d8/0x14a0 sound/usb/endpoint.c:1591
 start_endpoints+0xa1/0x280 sound/usb/pcm.c:291
 snd_usb_substream_playback_trigger+0x3e0/0x7a0 sound/usb/pcm.c:1711
 snd_pcm_do_start+0xb7/0x180 sound/core/pcm_native.c:1454
 snd_pcm_action_single sound/core/pcm_native.c:1310 [inline]
 snd_pcm_action+0xe7/0x240 sound/core/pcm_native.c:1393
 __snd_pcm_lib_xfer+0x1762/0x1ce0 sound/core/pcm_lib.c:2405
 snd_pcm_oss_write3+0x1bc/0x320 sound/core/oss/pcm_oss.c:1241
 snd_pcm_plug_write_transfer+0x2cb/0x4c0 sound/core/oss/pcm_plugin.c:630
 snd_pcm_oss_write2 sound/core/oss/pcm_oss.c:1373 [inline]
 snd_pcm_oss_write1 sound/core/oss/pcm_oss.c:1439 [inline]
 snd_pcm_oss_write+0xb9c/0x1190 sound/core/oss/pcm_oss.c:2794
 vfs_write+0x27e/0xb30 fs/read_write.c:684
 ksys_write+0x145/0x250 fs/read_write.c:738
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xfa/0xfa0 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7ff91798efc9
Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 a8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffee4c24fc8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00007ff917be5fa0 RCX: 00007ff91798efc9
RDX: 00000000000005ce RSI: 0000200000000e00 RDI: 0000000000000004
RBP: 00007ff917a11f91 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007ff917be5fa0 R14: 00007ff917be5fa0 R15: 0000000000000003
 </TASK>

Allocated by task 5461:
 kasan_save_stack mm/kasan/common.c:56 [inline]
 kasan_save_track+0x3e/0x80 mm/kasan/common.c:77
 poison_kmalloc_redzone mm/kasan/common.c:400 [inline]
 __kasan_kmalloc+0x93/0xb0 mm/kasan/common.c:417
 kasan_kmalloc include/linux/kasan.h:262 [inline]
 __do_kmalloc_node mm/slub.c:5642 [inline]
 __kmalloc_noprof+0x411/0x7f0 mm/slub.c:5654
 data_ep_set_params sound/usb/endpoint.c:1242 [inline]
 snd_usb_endpoint_set_params+0x1610/0x29a0 sound/usb/endpoint.c:1374
 snd_usb_hw_params+0xb12/0x1280 sound/usb/pcm.c:576
 snd_pcm_hw_params+0x89d/0x1d30 sound/core/pcm_native.c:804
 snd_pcm_oss_change_params_locked+0x21cb/0x3e40 sound/core/oss/pcm_oss.c:965
 snd_pcm_oss_make_ready_locked sound/core/oss/pcm_oss.c:1186 [inline]
 snd_pcm_oss_write1 sound/core/oss/pcm_oss.c:1404 [inline]
 snd_pcm_oss_write+0x2fb/0x1190 sound/core/oss/pcm_oss.c:2794
 vfs_write+0x27e/0xb30 fs/read_write.c:684
 ksys_write+0x145/0x250 fs/read_write.c:738
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0xfa/0xfa0 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

The buggy address belongs to the object at ffff88801107b400
 which belongs to the cache kmalloc-64 of size 64
The buggy address is located 0 bytes inside of
 allocated 54-byte region [ffff88801107b400, ffff88801107b436)

The buggy address belongs to the physical page:
page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x1107b
anon flags: 0xfff00000000000(node=0|zone=1|lastcpupid=0x7ff)
page_type: f5(slab)
raw: 00fff00000000000 ffff88801a0418c0 ffffea000045fc80 dead000000000005
raw: 0000000000000000 0000000080200020 00000000f5000000 0000000000000000
page dumped because: kasan: bad access detected
page_owner tracks the page as allocated
page last allocated via order 0, migratetype Unmovable, gfp_mask 0x52c40(GFP_NOFS|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP), pid 5341, tgid 5341 (syz-executor), ts 97923846546, free_ts 97738761799
 set_page_owner include/linux/page_owner.h:32 [inline]
 post_alloc_hook+0x240/0x2a0 mm/page_alloc.c:1850
 prep_new_page mm/page_alloc.c:1858 [inline]
 get_page_from_freelist+0x2365/0x2440 mm/page_alloc.c:3884
 __alloc_frozen_pages_noprof+0x181/0x370 mm/page_alloc.c:5183
 alloc_pages_mpol+0x232/0x4a0 mm/mempolicy.c:2416
 alloc_slab_page mm/slub.c:3055 [inline]
 allocate_slab+0x96/0x350 mm/slub.c:3228
 new_slab mm/slub.c:3282 [inline]
 ___slab_alloc+0xe94/0x18a0 mm/slub.c:4651
 __slab_alloc+0x65/0x100 mm/slub.c:4770
 __slab_alloc_node mm/slub.c:4846 [inline]
 slab_alloc_node mm/slub.c:5268 [inline]
 __do_kmalloc_node mm/slub.c:5641 [inline]
 __kmalloc_noprof+0x471/0x7f0 mm/slub.c:5654
 kmalloc_noprof include/linux/slab.h:961 [inline]
 kzalloc_noprof include/linux/slab.h:1094 [inline]
 tomoyo_encode2 security/tomoyo/realpath.c:45 [inline]
 tomoyo_encode+0x28b/0x550 security/tomoyo/realpath.c:80
 tomoyo_realpath_from_path+0x58d/0x5d0 security/tomoyo/realpath.c:283
 tomoyo_get_realpath security/tomoyo/file.c:151 [inline]
 tomoyo_check_open_permission+0x1c1/0x3b0 security/tomoyo/file.c:771
 security_file_open+0xb1/0x270 security/security.c:3183
 do_dentry_open+0x384/0x13f0 fs/open.c:942
 vfs_open+0x3b/0x340 fs/open.c:1097
 do_open fs/namei.c:3975 [inline]
 path_openat+0x2ee5/0x3830 fs/namei.c:4134
 do_filp_open+0x1fa/0x410 fs/namei.c:4161
page last free pid 5331 tgid 5331 stack trace:
 reset_page_owner include/linux/page_owner.h:25 [inline]
 free_pages_prepare mm/page_alloc.c:1394 [inline]
 __free_frozen_pages+0xbc4/0xd30 mm/page_alloc.c:2906
 vfree+0x25a/0x400 mm/vmalloc.c:3440
 kcov_put kernel/kcov.c:439 [inline]
 kcov_close+0x28/0x50 kernel/kcov.c:535
 __fput+0x44c/0xa70 fs/file_table.c:468
 task_work_run+0x1d4/0x260 kernel/task_work.c:227
 exit_task_work include/linux/task_work.h:40 [inline]
 do_exit+0x6b5/0x2300 kernel/exit.c:966
 do_group_exit+0x21c/0x2d0 kernel/exit.c:1107
 get_signal+0x1285/0x1340 kernel/signal.c:3034
 arch_do_signal_or_restart+0xa0/0x790 arch/x86/kernel/signal.c:337
 exit_to_user_mode_loop+0x72/0x130 kernel/entry/common.c:40
 exit_to_user_mode_prepare include/linux/irq-entry-common.h:225 [inline]
 syscall_exit_to_user_mode_work include/linux/entry-common.h:175 [inline]
 syscall_exit_to_user_mode include/linux/entry-common.h:210 [inline]
 do_syscall_64+0x2bd/0xfa0 arch/x86/entry/syscall_64.c:100
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Memory state around the buggy address:
 ffff88801107b300: 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc fc
 ffff88801107b380: 00 00 00 00 00 fc fc fc fc fc fc fc fc fc fc fc
>ffff88801107b400: 00 00 00 00 00 00 06 fc fc fc fc fc fc fc fc fc
                                     ^
 ffff88801107b480: 00 00 00 00 00 00 06 fc fc fc fc fc fc fc fc fc
 ffff88801107b500: fa fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
==================================================================


---
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] 16+ messages in thread

* Re: [syzbot] [sound?] [usb?] KASAN: slab-out-of-bounds Write in copy_to_urb
  2025-11-05 15:20 [syzbot] [sound?] [usb?] KASAN: slab-out-of-bounds Write in copy_to_urb syzbot
@ 2025-11-06  2:10 ` Hillf Danton
  2025-11-06  2:32   ` syzbot
  2025-11-06  7:50 ` Forwarded: " syzbot
  2025-11-06  8:33 ` [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds Lizhi Xu
  2 siblings, 1 reply; 16+ messages in thread
From: Hillf Danton @ 2025-11-06  2:10 UTC (permalink / raw)
  To: syzbot; +Cc: linux-kernel, syzkaller-bugs

> Date: Wed, 05 Nov 2025 07:20:38 -0800
> Hello,
> 
> syzbot found the following issue on:
> 
> HEAD commit:    ba36dd5ee6fd Merge tag 'bpf-fixes' of git://git.kernel.org..
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=1759afe2580000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=929790bc044e87d7
> dashboard link: https://syzkaller.appspot.com/bug?extid=bfd77469c8966de076f7
> compiler:       Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=11c74292580000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=107bffe2580000

#syz test

--- x/sound/usb/pcm.c
+++ y/sound/usb/pcm.c
@@ -1470,6 +1470,21 @@ static void fill_playback_urb_dsd_bitrev
 	urb_ctx_queue_advance(subs, urb, bytes);
 }
 
+static int copy_to_urb_prepare(struct snd_usb_substream *subs, struct urb *urb,
+			int offset, int stride, unsigned int bytes)
+{
+	if (subs->hwptr_done + bytes > subs->buffer_bytes) {
+		;
+	} else {
+		struct snd_urb_ctx *ctx = urb->context;
+
+		if (bytes > ctx->buffer_size ||
+		    offset > ctx->buffer_size - bytes)
+			return -EAGAIN;
+	}
+	return 0;
+}
+
 static void copy_to_urb(struct snd_usb_substream *subs, struct urb *urb,
 			int offset, int stride, unsigned int bytes)
 {
@@ -1607,9 +1622,12 @@ static int prepare_playback_urb(struct s
 			fill_playback_urb_dsd_bitrev(subs, urb, bytes);
 		} else {
 			/* usual PCM */
-			if (!subs->tx_length_quirk)
+			if (!subs->tx_length_quirk) {
+				int ret = copy_to_urb_prepare(subs, urb, 0, stride, bytes);
+				if (ret)
+					return ret;
 				copy_to_urb(subs, urb, 0, stride, bytes);
-			else
+			} else
 				bytes = copy_to_urb_quirk(subs, urb, stride, bytes);
 			/* bytes is now amount of outgoing data */
 		}
--

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

* Re: [syzbot] [sound?] [usb?] KASAN: slab-out-of-bounds Write in copy_to_urb
  2025-11-06  2:10 ` Hillf Danton
@ 2025-11-06  2:32   ` syzbot
  0 siblings, 0 replies; 16+ messages in thread
From: syzbot @ 2025-11-06  2:32 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-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
Tested-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com

Tested on:

commit:         dc77806c Merge tag 'rust-fixes-6.18' of git://git.kern..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1647a532580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=929790bc044e87d7
dashboard link: https://syzkaller.appspot.com/bug?extid=bfd77469c8966de076f7
compiler:       Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
patch:          https://syzkaller.appspot.com/x/patch.diff?x=16fd2012580000

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

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

* Forwarded: Re: [syzbot] [sound?] [usb?] KASAN: slab-out-of-bounds Write in copy_to_urb
  2025-11-05 15:20 [syzbot] [sound?] [usb?] KASAN: slab-out-of-bounds Write in copy_to_urb syzbot
  2025-11-06  2:10 ` Hillf Danton
@ 2025-11-06  7:50 ` syzbot
  2025-11-06  8:33 ` [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds Lizhi Xu
  2 siblings, 0 replies; 16+ messages in thread
From: syzbot @ 2025-11-06  7:50 UTC (permalink / raw)
  To: linux-kernel

For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org.

***

Subject: Re: [syzbot] [sound?] [usb?] KASAN: slab-out-of-bounds Write in copy_to_urb
Author: lizhi.xu@windriver.com

#syz test

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 54d01dfd820f..a4c0ea685b8a 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -1606,6 +1606,9 @@ static int prepare_playback_urb(struct snd_usb_substream *subs,
 				    subs->cur_audiofmt->dsd_bitrev)) {
 			fill_playback_urb_dsd_bitrev(subs, urb, bytes);
 		} else {
+			if (bytes > ctx->buffer_size)
+				return -EPIPE;
+
 			/* usual PCM */
 			if (!subs->tx_length_quirk)
 				copy_to_urb(subs, urb, 0, stride, bytes);

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

* Re: [syzbot] [sound?] [usb?] KASAN: slab-out-of-bounds Write in copy_to_urb
       [not found] <20251106074957.2742443-1-lizhi.xu@windriver.com>
@ 2025-11-06  8:11 ` syzbot
  0 siblings, 0 replies; 16+ messages in thread
From: syzbot @ 2025-11-06  8:11 UTC (permalink / raw)
  To: linux-kernel, lizhi.xu, syzkaller-bugs

Hello,

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

Reported-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
Tested-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com

Tested on:

commit:         dc77806c Merge tag 'rust-fixes-6.18' of git://git.kern..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=16335812580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=929790bc044e87d7
dashboard link: https://syzkaller.appspot.com/bug?extid=bfd77469c8966de076f7
compiler:       Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
patch:          https://syzkaller.appspot.com/x/patch.diff?x=12764a92580000

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

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

* [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds
  2025-11-05 15:20 [syzbot] [sound?] [usb?] KASAN: slab-out-of-bounds Write in copy_to_urb syzbot
  2025-11-06  2:10 ` Hillf Danton
  2025-11-06  7:50 ` Forwarded: " syzbot
@ 2025-11-06  8:33 ` Lizhi Xu
  2025-11-06 10:02   ` Takashi Iwai
  2 siblings, 1 reply; 16+ messages in thread
From: Lizhi Xu @ 2025-11-06  8:33 UTC (permalink / raw)
  To: syzbot+bfd77469c8966de076f7
  Cc: linux-kernel, linux-sound, linux-usb, perex, syzkaller-bugs,
	tiwai

The calculation rule for the actual data length written to the URB's
transfer buffer differs from that used to allocate the URB's transfer
buffer, and in this problem, the value used during allocation is smaller.

This ultimately leads to write out-of-bounds errors when writing data to
the transfer buffer.

To prevent out-of-bounds writes to the transfer buffer, a check between
the size of the bytes to be written and the size of the allocated bytes
should be added before performing the write operation.

When the written bytes are too large, -EPIPE is returned instead of
-EAGAIN, because returning -EAGAIN might result in push back to ready
list again.

Based on the context of calculating the bytes to be written here, both
copy_to_urb() and copy_to_urb_quirk() require a check for the size of
the bytes to be written before execution.

syzbot reported:
BUG: KASAN: slab-out-of-bounds in copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
Write of size 264 at addr ffff88801107b400 by task syz.0.17/5461

Call Trace:
 copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
 prepare_playback_urb+0x953/0x13d0 sound/usb/pcm.c:1611

Reported-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=bfd77469c8966de076f7
Tested-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
 sound/usb/pcm.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 54d01dfd820f..a4c0ea685b8a 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -1606,6 +1606,9 @@ static int prepare_playback_urb(struct snd_usb_substream *subs,
 				    subs->cur_audiofmt->dsd_bitrev)) {
 			fill_playback_urb_dsd_bitrev(subs, urb, bytes);
 		} else {
+			if (bytes > ctx->buffer_size)
+				return -EPIPE;
+
 			/* usual PCM */
 			if (!subs->tx_length_quirk)
 				copy_to_urb(subs, urb, 0, stride, bytes);
-- 
2.43.0


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

* Re: [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds
  2025-11-06  8:33 ` [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds Lizhi Xu
@ 2025-11-06 10:02   ` Takashi Iwai
  2025-11-06 11:31     ` Lizhi Xu
  0 siblings, 1 reply; 16+ messages in thread
From: Takashi Iwai @ 2025-11-06 10:02 UTC (permalink / raw)
  To: Lizhi Xu
  Cc: syzbot+bfd77469c8966de076f7, linux-kernel, linux-sound, linux-usb,
	perex, syzkaller-bugs, tiwai

On Thu, 06 Nov 2025 09:33:00 +0100,
Lizhi Xu wrote:
> 
> The calculation rule for the actual data length written to the URB's
> transfer buffer differs from that used to allocate the URB's transfer
> buffer, and in this problem, the value used during allocation is smaller.
> 
> This ultimately leads to write out-of-bounds errors when writing data to
> the transfer buffer.
> 
> To prevent out-of-bounds writes to the transfer buffer, a check between
> the size of the bytes to be written and the size of the allocated bytes
> should be added before performing the write operation.
> 
> When the written bytes are too large, -EPIPE is returned instead of
> -EAGAIN, because returning -EAGAIN might result in push back to ready
> list again.
> 
> Based on the context of calculating the bytes to be written here, both
> copy_to_urb() and copy_to_urb_quirk() require a check for the size of
> the bytes to be written before execution.
> 
> syzbot reported:
> BUG: KASAN: slab-out-of-bounds in copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
> Write of size 264 at addr ffff88801107b400 by task syz.0.17/5461
> 
> Call Trace:
>  copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
>  prepare_playback_urb+0x953/0x13d0 sound/usb/pcm.c:1611
> 
> Reported-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=bfd77469c8966de076f7
> Tested-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>

I'm afraid that this doesn't address the root cause at all.
The description above sounds plausible, but not pointing to "why".

The bytes is frames * stride, so the question is why a too large
frames is calculated.  I couldn't have time to check the details, but
there should be rather some weird condition / parameters to trigger
this, and we should check that at first.


thanks,

Takashi


> ---
>  sound/usb/pcm.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> index 54d01dfd820f..a4c0ea685b8a 100644
> --- a/sound/usb/pcm.c
> +++ b/sound/usb/pcm.c
> @@ -1606,6 +1606,9 @@ static int prepare_playback_urb(struct snd_usb_substream *subs,
>  				    subs->cur_audiofmt->dsd_bitrev)) {
>  			fill_playback_urb_dsd_bitrev(subs, urb, bytes);
>  		} else {
> +			if (bytes > ctx->buffer_size)
> +				return -EPIPE;
> +
>  			/* usual PCM */
>  			if (!subs->tx_length_quirk)
>  				copy_to_urb(subs, urb, 0, stride, bytes);
> -- 
> 2.43.0
> 

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

* Re: [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds
  2025-11-06 10:02   ` Takashi Iwai
@ 2025-11-06 11:31     ` Lizhi Xu
  2025-11-06 11:49       ` Takashi Iwai
  0 siblings, 1 reply; 16+ messages in thread
From: Lizhi Xu @ 2025-11-06 11:31 UTC (permalink / raw)
  To: tiwai
  Cc: linux-kernel, linux-sound, linux-usb, lizhi.xu, perex,
	syzbot+bfd77469c8966de076f7, syzkaller-bugs, tiwai

On Thu, 06 Nov 2025 11:02:08 +0100, Takashi Iwai wrote:
> > The calculation rule for the actual data length written to the URB's
> > transfer buffer differs from that used to allocate the URB's transfer
> > buffer, and in this problem, the value used during allocation is smaller.
> > 
> > This ultimately leads to write out-of-bounds errors when writing data to
> > the transfer buffer.
> > 
> > To prevent out-of-bounds writes to the transfer buffer, a check between
> > the size of the bytes to be written and the size of the allocated bytes
> > should be added before performing the write operation.
> > 
> > When the written bytes are too large, -EPIPE is returned instead of
> > -EAGAIN, because returning -EAGAIN might result in push back to ready
> > list again.
> > 
> > Based on the context of calculating the bytes to be written here, both
> > copy_to_urb() and copy_to_urb_quirk() require a check for the size of
> > the bytes to be written before execution.
> > 
> > syzbot reported:
> > BUG: KASAN: slab-out-of-bounds in copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
> > Write of size 264 at addr ffff88801107b400 by task syz.0.17/5461
> > 
> > Call Trace:
> >  copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
> >  prepare_playback_urb+0x953/0x13d0 sound/usb/pcm.c:1611
> > 
> > Reported-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=bfd77469c8966de076f7
> > Tested-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
> > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> 
> I'm afraid that this doesn't address the root cause at all.
> The description above sounds plausible, but not pointing to "why".
> 
> The bytes is frames * stride, so the question is why a too large
> frames is calculated.  I couldn't have time to check the details, but
> there should be rather some weird condition / parameters to trigger
> this, and we should check that at first.
During debugging, I discovered that the value of ep->packsize[0] is 22,
which causes the counts calculated by 
counts = snd_usb_endpoint_next_packet_size(ep, ctx, i, avail);
to be 22, resulting in a frames value of 22 * 6 = 132; 
Meanwhile, the stride value is 2, which ultimately results in 
bytes = frames * stride = 132 * 2 = 264;
@@ -1241,6 +1252,10 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep)
	u->buffer_size = maxsize * u->packets;
	...
	u->urb->transfer_buffer =
                usb_alloc_coherent(chip->dev, u->buffer_size,
                                   GFP_KERNEL, &u->urb->transfer_dma);

Here, when calculating u->buffer_size = maxsize * u->packets;
maxsize = 9, packets = 6, which results in only 54 bytes allocated to
transfer_buffer;

BR,
Lizhi
> > ---
> >  sound/usb/pcm.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
> > index 54d01dfd820f..a4c0ea685b8a 100644
> > --- a/sound/usb/pcm.c
> > +++ b/sound/usb/pcm.c
> > @@ -1606,6 +1606,9 @@ static int prepare_playback_urb(struct snd_usb_substream *subs,
> >  				    subs->cur_audiofmt->dsd_bitrev)) {
> >  			fill_playback_urb_dsd_bitrev(subs, urb, bytes);
> >  		} else {
> > +			if (bytes > ctx->buffer_size)
> > +				return -EPIPE;
> > +
> >  			/* usual PCM */
> >  			if (!subs->tx_length_quirk)
> >  				copy_to_urb(subs, urb, 0, stride, bytes);
> > --
> > 2.43.0
> >

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

* Re: [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds
  2025-11-06 11:31     ` Lizhi Xu
@ 2025-11-06 11:49       ` Takashi Iwai
  2025-11-06 14:35         ` Lizhi Xu
  0 siblings, 1 reply; 16+ messages in thread
From: Takashi Iwai @ 2025-11-06 11:49 UTC (permalink / raw)
  To: Lizhi Xu
  Cc: tiwai, linux-kernel, linux-sound, linux-usb, perex,
	syzbot+bfd77469c8966de076f7, syzkaller-bugs, tiwai

On Thu, 06 Nov 2025 12:31:21 +0100,
Lizhi Xu wrote:
> 
> On Thu, 06 Nov 2025 11:02:08 +0100, Takashi Iwai wrote:
> > > The calculation rule for the actual data length written to the URB's
> > > transfer buffer differs from that used to allocate the URB's transfer
> > > buffer, and in this problem, the value used during allocation is smaller.
> > > 
> > > This ultimately leads to write out-of-bounds errors when writing data to
> > > the transfer buffer.
> > > 
> > > To prevent out-of-bounds writes to the transfer buffer, a check between
> > > the size of the bytes to be written and the size of the allocated bytes
> > > should be added before performing the write operation.
> > > 
> > > When the written bytes are too large, -EPIPE is returned instead of
> > > -EAGAIN, because returning -EAGAIN might result in push back to ready
> > > list again.
> > > 
> > > Based on the context of calculating the bytes to be written here, both
> > > copy_to_urb() and copy_to_urb_quirk() require a check for the size of
> > > the bytes to be written before execution.
> > > 
> > > syzbot reported:
> > > BUG: KASAN: slab-out-of-bounds in copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
> > > Write of size 264 at addr ffff88801107b400 by task syz.0.17/5461
> > > 
> > > Call Trace:
> > >  copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
> > >  prepare_playback_urb+0x953/0x13d0 sound/usb/pcm.c:1611
> > > 
> > > Reported-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=bfd77469c8966de076f7
> > > Tested-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
> > > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> > 
> > I'm afraid that this doesn't address the root cause at all.
> > The description above sounds plausible, but not pointing to "why".
> > 
> > The bytes is frames * stride, so the question is why a too large
> > frames is calculated.  I couldn't have time to check the details, but
> > there should be rather some weird condition / parameters to trigger
> > this, and we should check that at first.
> During debugging, I discovered that the value of ep->packsize[0] is 22,
> which causes the counts calculated by 
> counts = snd_usb_endpoint_next_packet_size(ep, ctx, i, avail);
> to be 22, resulting in a frames value of 22 * 6 = 132; 
> Meanwhile, the stride value is 2, which ultimately results in 
> bytes = frames * stride = 132 * 2 = 264;
> @@ -1241,6 +1252,10 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep)
> 	u->buffer_size = maxsize * u->packets;
> 	...
> 	u->urb->transfer_buffer =
>                 usb_alloc_coherent(chip->dev, u->buffer_size,
>                                    GFP_KERNEL, &u->urb->transfer_dma);
> 
> Here, when calculating u->buffer_size = maxsize * u->packets;
> maxsize = 9, packets = 6, which results in only 54 bytes allocated to
> transfer_buffer;

Hm, so the problem is rather the calculation of the buffer size.
The size sounds extremely small.  Which parameters (rates, formats,
etc) are used for achieving this?

The calculation of u->buffer_size is a bit complex, as maxsize is
adjusted in many different ways.  Is it limited due to wMaxPacketSize
setup?


thanks,

Takashi

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

* Re: [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds
  2025-11-06 11:49       ` Takashi Iwai
@ 2025-11-06 14:35         ` Lizhi Xu
  2025-11-06 16:41           ` Takashi Iwai
  0 siblings, 1 reply; 16+ messages in thread
From: Lizhi Xu @ 2025-11-06 14:35 UTC (permalink / raw)
  To: tiwai
  Cc: linux-kernel, linux-sound, linux-usb, lizhi.xu, perex,
	syzbot+bfd77469c8966de076f7, syzkaller-bugs, tiwai

On Thu, 06 Nov 2025 12:49:51 +0100, Takashi Iwai wrote:
> > > > The calculation rule for the actual data length written to the URB's
> > > > transfer buffer differs from that used to allocate the URB's transfer
> > > > buffer, and in this problem, the value used during allocation is smaller.
> > > >
> > > > This ultimately leads to write out-of-bounds errors when writing data to
> > > > the transfer buffer.
> > > >
> > > > To prevent out-of-bounds writes to the transfer buffer, a check between
> > > > the size of the bytes to be written and the size of the allocated bytes
> > > > should be added before performing the write operation.
> > > >
> > > > When the written bytes are too large, -EPIPE is returned instead of
> > > > -EAGAIN, because returning -EAGAIN might result in push back to ready
> > > > list again.
> > > >
> > > > Based on the context of calculating the bytes to be written here, both
> > > > copy_to_urb() and copy_to_urb_quirk() require a check for the size of
> > > > the bytes to be written before execution.
> > > >
> > > > syzbot reported:
> > > > BUG: KASAN: slab-out-of-bounds in copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
> > > > Write of size 264 at addr ffff88801107b400 by task syz.0.17/5461
> > > >
> > > > Call Trace:
> > > >  copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
> > > >  prepare_playback_urb+0x953/0x13d0 sound/usb/pcm.c:1611
> > > >
> > > > Reported-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
> > > > Closes: https://syzkaller.appspot.com/bug?extid=bfd77469c8966de076f7
> > > > Tested-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
> > > > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> > >
> > > I'm afraid that this doesn't address the root cause at all.
> > > The description above sounds plausible, but not pointing to "why".
> > >
> > > The bytes is frames * stride, so the question is why a too large
> > > frames is calculated.  I couldn't have time to check the details, but
> > > there should be rather some weird condition / parameters to trigger
> > > this, and we should check that at first.
> > During debugging, I discovered that the value of ep->packsize[0] is 22,
> > which causes the counts calculated by
> > counts = snd_usb_endpoint_next_packet_size(ep, ctx, i, avail);
> > to be 22, resulting in a frames value of 22 * 6 = 132;
> > Meanwhile, the stride value is 2, which ultimately results in
> > bytes = frames * stride = 132 * 2 = 264;
> > @@ -1241,6 +1252,10 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep)
> > 	u->buffer_size = maxsize * u->packets;
> > 	...
> > 	u->urb->transfer_buffer =
> >                 usb_alloc_coherent(chip->dev, u->buffer_size,
> >                                    GFP_KERNEL, &u->urb->transfer_dma);
> >
> > Here, when calculating u->buffer_size = maxsize * u->packets;
> > maxsize = 9, packets = 6, which results in only 54 bytes allocated to
> > transfer_buffer;
> 
> Hm, so the problem is rather the calculation of the buffer size.
> The size sounds extremely small.  Which parameters (rates, formats,
> etc) are used for achieving this?
rates: 22050
format: 2
channels: 1
/////////////////////////////
stride: 2
packets: 6
data interval: 0
frame_bits: 16
> 
> The calculation of u->buffer_size is a bit complex, as maxsize is
> adjusted in many different ways.  Is it limited due to wMaxPacketSize
> setup?
Yes, it's because the value of ep->maxpacksize is 9 that the maxsize
value is 9.

BR,
Lizhi

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

* Re: [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds
  2025-11-06 14:35         ` Lizhi Xu
@ 2025-11-06 16:41           ` Takashi Iwai
  2025-11-06 23:51             ` Hillf Danton
  2025-11-07  0:54             ` [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds Lizhi Xu
  0 siblings, 2 replies; 16+ messages in thread
From: Takashi Iwai @ 2025-11-06 16:41 UTC (permalink / raw)
  To: Lizhi Xu
  Cc: tiwai, linux-kernel, linux-sound, linux-usb, perex,
	syzbot+bfd77469c8966de076f7, syzkaller-bugs, tiwai

On Thu, 06 Nov 2025 15:35:06 +0100,
Lizhi Xu wrote:
> 
> On Thu, 06 Nov 2025 12:49:51 +0100, Takashi Iwai wrote:
> > > > > The calculation rule for the actual data length written to the URB's
> > > > > transfer buffer differs from that used to allocate the URB's transfer
> > > > > buffer, and in this problem, the value used during allocation is smaller.
> > > > >
> > > > > This ultimately leads to write out-of-bounds errors when writing data to
> > > > > the transfer buffer.
> > > > >
> > > > > To prevent out-of-bounds writes to the transfer buffer, a check between
> > > > > the size of the bytes to be written and the size of the allocated bytes
> > > > > should be added before performing the write operation.
> > > > >
> > > > > When the written bytes are too large, -EPIPE is returned instead of
> > > > > -EAGAIN, because returning -EAGAIN might result in push back to ready
> > > > > list again.
> > > > >
> > > > > Based on the context of calculating the bytes to be written here, both
> > > > > copy_to_urb() and copy_to_urb_quirk() require a check for the size of
> > > > > the bytes to be written before execution.
> > > > >
> > > > > syzbot reported:
> > > > > BUG: KASAN: slab-out-of-bounds in copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
> > > > > Write of size 264 at addr ffff88801107b400 by task syz.0.17/5461
> > > > >
> > > > > Call Trace:
> > > > >  copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
> > > > >  prepare_playback_urb+0x953/0x13d0 sound/usb/pcm.c:1611
> > > > >
> > > > > Reported-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
> > > > > Closes: https://syzkaller.appspot.com/bug?extid=bfd77469c8966de076f7
> > > > > Tested-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
> > > > > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> > > >
> > > > I'm afraid that this doesn't address the root cause at all.
> > > > The description above sounds plausible, but not pointing to "why".
> > > >
> > > > The bytes is frames * stride, so the question is why a too large
> > > > frames is calculated.  I couldn't have time to check the details, but
> > > > there should be rather some weird condition / parameters to trigger
> > > > this, and we should check that at first.
> > > During debugging, I discovered that the value of ep->packsize[0] is 22,
> > > which causes the counts calculated by
> > > counts = snd_usb_endpoint_next_packet_size(ep, ctx, i, avail);
> > > to be 22, resulting in a frames value of 22 * 6 = 132;
> > > Meanwhile, the stride value is 2, which ultimately results in
> > > bytes = frames * stride = 132 * 2 = 264;
> > > @@ -1241,6 +1252,10 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep)
> > > 	u->buffer_size = maxsize * u->packets;
> > > 	...
> > > 	u->urb->transfer_buffer =
> > >                 usb_alloc_coherent(chip->dev, u->buffer_size,
> > >                                    GFP_KERNEL, &u->urb->transfer_dma);
> > >
> > > Here, when calculating u->buffer_size = maxsize * u->packets;
> > > maxsize = 9, packets = 6, which results in only 54 bytes allocated to
> > > transfer_buffer;
> > 
> > Hm, so the problem is rather the calculation of the buffer size.
> > The size sounds extremely small.  Which parameters (rates, formats,
> > etc) are used for achieving this?
> rates: 22050
> format: 2
> channels: 1
> /////////////////////////////
> stride: 2
> packets: 6
> data interval: 0
> frame_bits: 16
> > 
> > The calculation of u->buffer_size is a bit complex, as maxsize is
> > adjusted in many different ways.  Is it limited due to wMaxPacketSize
> > setup?
> Yes, it's because the value of ep->maxpacksize is 9 that the maxsize
> value is 9.

OK, then a fix like below would work?


thanks,

Takashi

--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -1362,6 +1362,11 @@ int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
 	ep->sample_rem = ep->cur_rate % ep->pps;
 	ep->packsize[0] = ep->cur_rate / ep->pps;
 	ep->packsize[1] = (ep->cur_rate + (ep->pps - 1)) / ep->pps;
+	if (ep->packsize[1] > ep->maxpacksize) {
+		usb_audio_dbg(chip, "Too small maxpacksize %u for rate %u / pps %u\n",
+			      ep->maxpacksize, ep->cur_rate, ep->pps);
+		return -EINVAL;
+	}
 
 	/* calculate the frequency in 16.16 format */
 	ep->freqm = ep->freqn;

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

* Re: [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds
  2025-11-06 16:41           ` Takashi Iwai
@ 2025-11-06 23:51             ` Hillf Danton
  2025-11-07  0:12               ` [syzbot] [sound?] [usb?] KASAN: slab-out-of-bounds Write in copy_to_urb syzbot
  2025-11-07  0:54             ` [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds Lizhi Xu
  1 sibling, 1 reply; 16+ messages in thread
From: Hillf Danton @ 2025-11-06 23:51 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Lizhi Xu, linux-kernel, linux-sound, linux-usb, perex,
	syzbot+bfd77469c8966de076f7, syzkaller-bugs, tiwai

On Thu, 06 Nov 2025 17:41:07 +0100 Takashi Iwai wrote:
> OK, then a fix like below would work?

Test Takashi's fix.

#syz test

--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -1362,6 +1362,11 @@ int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
 	ep->sample_rem = ep->cur_rate % ep->pps;
 	ep->packsize[0] = ep->cur_rate / ep->pps;
 	ep->packsize[1] = (ep->cur_rate + (ep->pps - 1)) / ep->pps;
+	if (ep->packsize[1] > ep->maxpacksize) {
+		usb_audio_dbg(chip, "Too small maxpacksize %u for rate %u / pps %u\n",
+			      ep->maxpacksize, ep->cur_rate, ep->pps);
+		return -EINVAL;
+	}
 
 	/* calculate the frequency in 16.16 format */
 	ep->freqm = ep->freqn;
--

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

* Re: [syzbot] [sound?] [usb?] KASAN: slab-out-of-bounds Write in copy_to_urb
  2025-11-06 23:51             ` Hillf Danton
@ 2025-11-07  0:12               ` syzbot
  0 siblings, 0 replies; 16+ messages in thread
From: syzbot @ 2025-11-07  0:12 UTC (permalink / raw)
  To: hdanton, linux-kernel, linux-sound, linux-usb, lizhi.xu, perex,
	syzkaller-bugs, tiwai, tiwai

Hello,

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

Reported-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
Tested-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com

Tested on:

commit:         a1388fcb Merge tag 'libcrypto-for-linus' of git://git...
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1581d812580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=929790bc044e87d7
dashboard link: https://syzkaller.appspot.com/bug?extid=bfd77469c8966de076f7
compiler:       Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
patch:          https://syzkaller.appspot.com/x/patch.diff?x=10ae2a58580000

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

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

* Re: [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds
  2025-11-06 16:41           ` Takashi Iwai
  2025-11-06 23:51             ` Hillf Danton
@ 2025-11-07  0:54             ` Lizhi Xu
  2025-11-07  1:13               ` Lizhi Xu
  1 sibling, 1 reply; 16+ messages in thread
From: Lizhi Xu @ 2025-11-07  0:54 UTC (permalink / raw)
  To: tiwai
  Cc: hdanton, linux-kernel, linux-sound, linux-usb, lizhi.xu, perex,
	syzbot+bfd77469c8966de076f7, syzkaller-bugs

On Thu, 06 Nov 2025 17:41:07 +0100, Takashi Iwai wrote:
> > > > > > The calculation rule for the actual data length written to the URB's
> > > > > > transfer buffer differs from that used to allocate the URB's transfer
> > > > > > buffer, and in this problem, the value used during allocation is smaller.
> > > > > >
> > > > > > This ultimately leads to write out-of-bounds errors when writing data to
> > > > > > the transfer buffer.
> > > > > >
> > > > > > To prevent out-of-bounds writes to the transfer buffer, a check between
> > > > > > the size of the bytes to be written and the size of the allocated bytes
> > > > > > should be added before performing the write operation.
> > > > > >
> > > > > > When the written bytes are too large, -EPIPE is returned instead of
> > > > > > -EAGAIN, because returning -EAGAIN might result in push back to ready
> > > > > > list again.
> > > > > >
> > > > > > Based on the context of calculating the bytes to be written here, both
> > > > > > copy_to_urb() and copy_to_urb_quirk() require a check for the size of
> > > > > > the bytes to be written before execution.
> > > > > >
> > > > > > syzbot reported:
> > > > > > BUG: KASAN: slab-out-of-bounds in copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
> > > > > > Write of size 264 at addr ffff88801107b400 by task syz.0.17/5461
> > > > > >
> > > > > > Call Trace:
> > > > > >  copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
> > > > > >  prepare_playback_urb+0x953/0x13d0 sound/usb/pcm.c:1611
> > > > > >
> > > > > > Reported-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
> > > > > > Closes: https://syzkaller.appspot.com/bug?extid=bfd77469c8966de076f7
> > > > > > Tested-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
> > > > > > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> > > > >
> > > > > I'm afraid that this doesn't address the root cause at all.
> > > > > The description above sounds plausible, but not pointing to "why".
> > > > >
> > > > > The bytes is frames * stride, so the question is why a too large
> > > > > frames is calculated.  I couldn't have time to check the details, but
> > > > > there should be rather some weird condition / parameters to trigger
> > > > > this, and we should check that at first.
> > > > During debugging, I discovered that the value of ep->packsize[0] is 22,
> > > > which causes the counts calculated by
> > > > counts = snd_usb_endpoint_next_packet_size(ep, ctx, i, avail);
> > > > to be 22, resulting in a frames value of 22 * 6 = 132;
> > > > Meanwhile, the stride value is 2, which ultimately results in
> > > > bytes = frames * stride = 132 * 2 = 264;
> > > > @@ -1241,6 +1252,10 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep)
> > > > 	u->buffer_size = maxsize * u->packets;
> > > > 	...
> > > > 	u->urb->transfer_buffer =
> > > >                 usb_alloc_coherent(chip->dev, u->buffer_size,
> > > >                                    GFP_KERNEL, &u->urb->transfer_dma);
> > > >
> > > > Here, when calculating u->buffer_size = maxsize * u->packets;
> > > > maxsize = 9, packets = 6, which results in only 54 bytes allocated to
> > > > transfer_buffer;
> > >
> > > Hm, so the problem is rather the calculation of the buffer size.
> > > The size sounds extremely small.  Which parameters (rates, formats,
> > > etc) are used for achieving this?
> > rates: 22050
> > format: 2
> > channels: 1
> > /////////////////////////////
> > stride: 2
> > packets: 6
> > data interval: 0
> > frame_bits: 16
> > >
> > > The calculation of u->buffer_size is a bit complex, as maxsize is
> > > adjusted in many different ways.  Is it limited due to wMaxPacketSize
> > > setup?
> > Yes, it's because the value of ep->maxpacksize is 9 that the maxsize
> > value is 9.
> 
> OK, then a fix like below would work?
> 
> 
> thanks,
> 
> Takashi
> 
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -1362,6 +1362,11 @@ int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
>  	ep->sample_rem = ep->cur_rate % ep->pps;
>  	ep->packsize[0] = ep->cur_rate / ep->pps;
>  	ep->packsize[1] = (ep->cur_rate + (ep->pps - 1)) / ep->pps;
> +	if (ep->packsize[1] > ep->maxpacksize) {
> +		usb_audio_dbg(chip, "Too small maxpacksize %u for rate %u / pps %u\n",
> +			      ep->maxpacksize, ep->cur_rate, ep->pps);
> +		return -EINVAL;
> +	}
> 
>  	/* calculate the frequency in 16.16 format */
>  	ep->freqm = ep->freqn;
Of course, this fix was added after packsize[0] was assigned a value,
and Hillf Danton has already tested it.

However, to be more precise, although both packsize[1] and packsize[0]
exceed maxpacksize, this example is about packsize[0], so judging packsize[0]
is more rigorous.

BR,
Lizhi

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

* Re: [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds
  2025-11-07  0:54             ` [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds Lizhi Xu
@ 2025-11-07  1:13               ` Lizhi Xu
  2025-11-08  7:54                 ` Takashi Iwai
  0 siblings, 1 reply; 16+ messages in thread
From: Lizhi Xu @ 2025-11-07  1:13 UTC (permalink / raw)
  To: lizhi.xu
  Cc: tiwai, hdanton, linux-kernel, linux-sound, linux-usb, perex,
	syzbot+bfd77469c8966de076f7, syzkaller-bugs

On Fri, 7 Nov 2025 08:54:20 +0800, Lizhi Xu wrote:
> > > > > > > The calculation rule for the actual data length written to the URB's
> > > > > > > transfer buffer differs from that used to allocate the URB's transfer
> > > > > > > buffer, and in this problem, the value used during allocation is smaller.
> > > > > > >
> > > > > > > This ultimately leads to write out-of-bounds errors when writing data to
> > > > > > > the transfer buffer.
> > > > > > >
> > > > > > > To prevent out-of-bounds writes to the transfer buffer, a check between
> > > > > > > the size of the bytes to be written and the size of the allocated bytes
> > > > > > > should be added before performing the write operation.
> > > > > > >
> > > > > > > When the written bytes are too large, -EPIPE is returned instead of
> > > > > > > -EAGAIN, because returning -EAGAIN might result in push back to ready
> > > > > > > list again.
> > > > > > >
> > > > > > > Based on the context of calculating the bytes to be written here, both
> > > > > > > copy_to_urb() and copy_to_urb_quirk() require a check for the size of
> > > > > > > the bytes to be written before execution.
> > > > > > >
> > > > > > > syzbot reported:
> > > > > > > BUG: KASAN: slab-out-of-bounds in copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
> > > > > > > Write of size 264 at addr ffff88801107b400 by task syz.0.17/5461
> > > > > > >
> > > > > > > Call Trace:
> > > > > > >  copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
> > > > > > >  prepare_playback_urb+0x953/0x13d0 sound/usb/pcm.c:1611
> > > > > > >
> > > > > > > Reported-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
> > > > > > > Closes: https://syzkaller.appspot.com/bug?extid=bfd77469c8966de076f7
> > > > > > > Tested-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
> > > > > > > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> > > > > >
> > > > > > I'm afraid that this doesn't address the root cause at all.
> > > > > > The description above sounds plausible, but not pointing to "why".
> > > > > >
> > > > > > The bytes is frames * stride, so the question is why a too large
> > > > > > frames is calculated.  I couldn't have time to check the details, but
> > > > > > there should be rather some weird condition / parameters to trigger
> > > > > > this, and we should check that at first.
> > > > > During debugging, I discovered that the value of ep->packsize[0] is 22,
> > > > > which causes the counts calculated by
> > > > > counts = snd_usb_endpoint_next_packet_size(ep, ctx, i, avail);
> > > > > to be 22, resulting in a frames value of 22 * 6 = 132;
> > > > > Meanwhile, the stride value is 2, which ultimately results in
> > > > > bytes = frames * stride = 132 * 2 = 264;
> > > > > @@ -1241,6 +1252,10 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep)
> > > > > 	u->buffer_size = maxsize * u->packets;
> > > > > 	...
> > > > > 	u->urb->transfer_buffer =
> > > > >                 usb_alloc_coherent(chip->dev, u->buffer_size,
> > > > >                                    GFP_KERNEL, &u->urb->transfer_dma);
> > > > >
> > > > > Here, when calculating u->buffer_size = maxsize * u->packets;
> > > > > maxsize = 9, packets = 6, which results in only 54 bytes allocated to
> > > > > transfer_buffer;
> > > >
> > > > Hm, so the problem is rather the calculation of the buffer size.
> > > > The size sounds extremely small.  Which parameters (rates, formats,
> > > > etc) are used for achieving this?
> > > rates: 22050
> > > format: 2
> > > channels: 1
> > > /////////////////////////////
> > > stride: 2
> > > packets: 6
> > > data interval: 0
> > > frame_bits: 16
> > > >
> > > > The calculation of u->buffer_size is a bit complex, as maxsize is
> > > > adjusted in many different ways.  Is it limited due to wMaxPacketSize
> > > > setup?
> > > Yes, it's because the value of ep->maxpacksize is 9 that the maxsize
> > > value is 9.
> > 
> > OK, then a fix like below would work?
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> > --- a/sound/usb/endpoint.c
> > +++ b/sound/usb/endpoint.c
> > @@ -1362,6 +1362,11 @@ int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
> >  	ep->sample_rem = ep->cur_rate % ep->pps;
> >  	ep->packsize[0] = ep->cur_rate / ep->pps;
> >  	ep->packsize[1] = (ep->cur_rate + (ep->pps - 1)) / ep->pps;
> > +	if (ep->packsize[1] > ep->maxpacksize) {
> > +		usb_audio_dbg(chip, "Too small maxpacksize %u for rate %u / pps %u\n",
> > +			      ep->maxpacksize, ep->cur_rate, ep->pps);
> > +		return -EINVAL;
> > +	}
> > 
> >  	/* calculate the frequency in 16.16 format */
> >  	ep->freqm = ep->freqn;
> Of course, this fix was added after packsize[0] was assigned a value,
> and Hillf Danton has already tested it.
> 
> However, to be more precise, although both packsize[1] and packsize[0]
> exceed maxpacksize, this example is about packsize[0], so judging packsize[0]
> is more rigorous.
Of course, since packsize[1] is always greater than packsize[0] when pps
is greater than 1, judging only packsize[1] is sufficient to avoid judging
packsize[0] and packsize[1].

BR,
Lizhi

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

* Re: [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds
  2025-11-07  1:13               ` Lizhi Xu
@ 2025-11-08  7:54                 ` Takashi Iwai
  0 siblings, 0 replies; 16+ messages in thread
From: Takashi Iwai @ 2025-11-08  7:54 UTC (permalink / raw)
  To: Lizhi Xu
  Cc: tiwai, hdanton, linux-kernel, linux-sound, linux-usb, perex,
	syzbot+bfd77469c8966de076f7, syzkaller-bugs

On Fri, 07 Nov 2025 02:13:27 +0100,
Lizhi Xu wrote:
> 
> On Fri, 7 Nov 2025 08:54:20 +0800, Lizhi Xu wrote:
> > > > > > > > The calculation rule for the actual data length written to the URB's
> > > > > > > > transfer buffer differs from that used to allocate the URB's transfer
> > > > > > > > buffer, and in this problem, the value used during allocation is smaller.
> > > > > > > >
> > > > > > > > This ultimately leads to write out-of-bounds errors when writing data to
> > > > > > > > the transfer buffer.
> > > > > > > >
> > > > > > > > To prevent out-of-bounds writes to the transfer buffer, a check between
> > > > > > > > the size of the bytes to be written and the size of the allocated bytes
> > > > > > > > should be added before performing the write operation.
> > > > > > > >
> > > > > > > > When the written bytes are too large, -EPIPE is returned instead of
> > > > > > > > -EAGAIN, because returning -EAGAIN might result in push back to ready
> > > > > > > > list again.
> > > > > > > >
> > > > > > > > Based on the context of calculating the bytes to be written here, both
> > > > > > > > copy_to_urb() and copy_to_urb_quirk() require a check for the size of
> > > > > > > > the bytes to be written before execution.
> > > > > > > >
> > > > > > > > syzbot reported:
> > > > > > > > BUG: KASAN: slab-out-of-bounds in copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
> > > > > > > > Write of size 264 at addr ffff88801107b400 by task syz.0.17/5461
> > > > > > > >
> > > > > > > > Call Trace:
> > > > > > > >  copy_to_urb+0x261/0x460 sound/usb/pcm.c:1487
> > > > > > > >  prepare_playback_urb+0x953/0x13d0 sound/usb/pcm.c:1611
> > > > > > > >
> > > > > > > > Reported-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
> > > > > > > > Closes: https://syzkaller.appspot.com/bug?extid=bfd77469c8966de076f7
> > > > > > > > Tested-by: syzbot+bfd77469c8966de076f7@syzkaller.appspotmail.com
> > > > > > > > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> > > > > > >
> > > > > > > I'm afraid that this doesn't address the root cause at all.
> > > > > > > The description above sounds plausible, but not pointing to "why".
> > > > > > >
> > > > > > > The bytes is frames * stride, so the question is why a too large
> > > > > > > frames is calculated.  I couldn't have time to check the details, but
> > > > > > > there should be rather some weird condition / parameters to trigger
> > > > > > > this, and we should check that at first.
> > > > > > During debugging, I discovered that the value of ep->packsize[0] is 22,
> > > > > > which causes the counts calculated by
> > > > > > counts = snd_usb_endpoint_next_packet_size(ep, ctx, i, avail);
> > > > > > to be 22, resulting in a frames value of 22 * 6 = 132;
> > > > > > Meanwhile, the stride value is 2, which ultimately results in
> > > > > > bytes = frames * stride = 132 * 2 = 264;
> > > > > > @@ -1241,6 +1252,10 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep)
> > > > > > 	u->buffer_size = maxsize * u->packets;
> > > > > > 	...
> > > > > > 	u->urb->transfer_buffer =
> > > > > >                 usb_alloc_coherent(chip->dev, u->buffer_size,
> > > > > >                                    GFP_KERNEL, &u->urb->transfer_dma);
> > > > > >
> > > > > > Here, when calculating u->buffer_size = maxsize * u->packets;
> > > > > > maxsize = 9, packets = 6, which results in only 54 bytes allocated to
> > > > > > transfer_buffer;
> > > > >
> > > > > Hm, so the problem is rather the calculation of the buffer size.
> > > > > The size sounds extremely small.  Which parameters (rates, formats,
> > > > > etc) are used for achieving this?
> > > > rates: 22050
> > > > format: 2
> > > > channels: 1
> > > > /////////////////////////////
> > > > stride: 2
> > > > packets: 6
> > > > data interval: 0
> > > > frame_bits: 16
> > > > >
> > > > > The calculation of u->buffer_size is a bit complex, as maxsize is
> > > > > adjusted in many different ways.  Is it limited due to wMaxPacketSize
> > > > > setup?
> > > > Yes, it's because the value of ep->maxpacksize is 9 that the maxsize
> > > > value is 9.
> > > 
> > > OK, then a fix like below would work?
> > > 
> > > 
> > > thanks,
> > > 
> > > Takashi
> > > 
> > > --- a/sound/usb/endpoint.c
> > > +++ b/sound/usb/endpoint.c
> > > @@ -1362,6 +1362,11 @@ int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
> > >  	ep->sample_rem = ep->cur_rate % ep->pps;
> > >  	ep->packsize[0] = ep->cur_rate / ep->pps;
> > >  	ep->packsize[1] = (ep->cur_rate + (ep->pps - 1)) / ep->pps;
> > > +	if (ep->packsize[1] > ep->maxpacksize) {
> > > +		usb_audio_dbg(chip, "Too small maxpacksize %u for rate %u / pps %u\n",
> > > +			      ep->maxpacksize, ep->cur_rate, ep->pps);
> > > +		return -EINVAL;
> > > +	}
> > > 
> > >  	/* calculate the frequency in 16.16 format */
> > >  	ep->freqm = ep->freqn;
> > Of course, this fix was added after packsize[0] was assigned a value,
> > and Hillf Danton has already tested it.
> > 
> > However, to be more precise, although both packsize[1] and packsize[0]
> > exceed maxpacksize, this example is about packsize[0], so judging packsize[0]
> > is more rigorous.
> Of course, since packsize[1] is always greater than packsize[0] when pps
> is greater than 1, judging only packsize[1] is sufficient to avoid judging
> packsize[0] and packsize[1].

Right, that's the reason it checks only packsize[1].

I think it's fine to add another check like your patch in addition for
more safety, too.  But then it should better in copy_to_urb() for
both size and offset.  And, it should be with WARN_ON() or such, as
the OOB must not happen after packsize[] sanity check.


thanks,

Takashi

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

end of thread, other threads:[~2025-11-08  7:54 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-05 15:20 [syzbot] [sound?] [usb?] KASAN: slab-out-of-bounds Write in copy_to_urb syzbot
2025-11-06  2:10 ` Hillf Danton
2025-11-06  2:32   ` syzbot
2025-11-06  7:50 ` Forwarded: " syzbot
2025-11-06  8:33 ` [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds Lizhi Xu
2025-11-06 10:02   ` Takashi Iwai
2025-11-06 11:31     ` Lizhi Xu
2025-11-06 11:49       ` Takashi Iwai
2025-11-06 14:35         ` Lizhi Xu
2025-11-06 16:41           ` Takashi Iwai
2025-11-06 23:51             ` Hillf Danton
2025-11-07  0:12               ` [syzbot] [sound?] [usb?] KASAN: slab-out-of-bounds Write in copy_to_urb syzbot
2025-11-07  0:54             ` [PATCH] ALSA: usb-audio: Prevent urb from writing out of bounds Lizhi Xu
2025-11-07  1:13               ` Lizhi Xu
2025-11-08  7:54                 ` Takashi Iwai
     [not found] <20251106074957.2742443-1-lizhi.xu@windriver.com>
2025-11-06  8:11 ` [syzbot] [sound?] [usb?] KASAN: slab-out-of-bounds Write in copy_to_urb syzbot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox