Bug: slab-out-of-bounds in snd_seq_oss_synth_sysex

Bug: slab-out-of-bounds in snd_seq_oss_synth_sysex

[Kun Hu]

Hello,

When using fuzzer tool to fuzz the latest Linux kernel, the following crash
was triggered.

HEAD commit: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
git tree: upstream
Console output:https://drive.google.com/file/d/17oCyKDW_kNhSW5Bbvm23vnpD1eo0MHFi/view?usp=sharing
Kernel config: https://drive.google.com/file/d/1RhT5dFTs6Vx1U71PbpenN7TPtnPoa3NI/view?usp=sharing
C reproducer: https://drive.google.com/file/d/177HJht6a7-6F3YLudKb_d4kiPGd1VA_i/view?usp=sharing
Syzlang reproducer: https://drive.google.com/file/d/1AuP5UGGc47rEXXPuvjmCKgJ3d0U1P84j/view?usp=sharing


If you fix this issue, please add the following tag to the commit:
Reported-by: Kun Hu <huk23@m.fudan.edu.cn>

==================================================================
BUG: KASAN: slab-out-of-bounds in snd_seq_oss_synth_sysex+0x5d1/0x6c0 sound/core/seq/oss/seq_oss_synth.c:516
Write of size 1 at addr ff1100000588e288 by task syz-executor411/824

CPU: 2 UID: 0 PID: 824 Comm: syz-executor411 Not tainted 6.13.0-rc3 #5
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:94 [inline]
 dump_stack_lvl+0x116/0x1b0 lib/dump_stack.c:120
 print_address_description mm/kasan/report.c:378 [inline]
 print_report+0xcf/0x5f0 mm/kasan/report.c:489
 kasan_report+0x93/0xc0 mm/kasan/report.c:602
 snd_seq_oss_synth_sysex+0x5d1/0x6c0 sound/core/seq/oss/seq_oss_synth.c:516
 snd_seq_oss_process_event+0x46a/0x2620 sound/core/seq/oss/seq_oss_event.c:61
 insert_queue sound/core/seq/oss/seq_oss_rw.c:167 [inline]
 snd_seq_oss_write+0x261/0x7f0 sound/core/seq/oss/seq_oss_rw.c:135
 odev_write+0x53/0xa0 sound/core/seq/oss/seq_oss.c:168
 vfs_write fs/read_write.c:677 [inline]
 vfs_write+0x2e3/0x10f0 fs/read_write.c:659
 ksys_write+0x122/0x240 fs/read_write.c:731
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xc3/0x1d0 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f69b379994d
Code: c3 e8 97 2a 00 00 0f 1f 80 00 00 00 00 f3 0f 1e fa 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 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f69b3718d78 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00007f69b382d2d0 RCX: 00007f69b379994d
RDX: 0000000000000008 RSI: 0000000020000140 RDI: 0000000000000003
RBP: 00007f69b382d2d8 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f69b382d2dc
R13: 0000000020000140 R14: 00007f69b37fa008 R15: 000000000000000d
 </TASK>

Allocated by task 823:
 kasan_save_stack+0x24/0x50 mm/kasan/common.c:47
 kasan_save_track+0x14/0x30 mm/kasan/common.c:68
 poison_kmalloc_redzone mm/kasan/common.c:377 [inline]
 __kasan_kmalloc+0xaa/0xb0 mm/kasan/common.c:394
 kmalloc_noprof include/linux/slab.h:901 [inline]
 kzalloc_noprof include/linux/slab.h:1037 [inline]
 snd_seq_oss_synth_sysex+0x1d9/0x6c0 sound/core/seq/oss/seq_oss_synth.c:502
 snd_seq_oss_process_event+0x46a/0x2620 sound/core/seq/oss/seq_oss_event.c:61
 insert_queue sound/core/seq/oss/seq_oss_rw.c:167 [inline]
 snd_seq_oss_write+0x261/0x7f0 sound/core/seq/oss/seq_oss_rw.c:135
 odev_write+0x53/0xa0 sound/core/seq/oss/seq_oss.c:168
 vfs_write fs/read_write.c:677 [inline]
 vfs_write+0x2e3/0x10f0 fs/read_write.c:659
 ksys_write+0x122/0x240 fs/read_write.c:731
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xc3/0x1d0 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

The buggy address belongs to the object at ff1100000588e200
 which belongs to the cache kmalloc-192 of size 192
The buggy address is located 0 bytes to the right of
 allocated 136-byte region [ff1100000588e200, ff1100000588e288)

The buggy address belongs to the physical page:
page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x588e
anon flags: 0x100000000000000(node=0|zone=1)
page_type: f5(slab)
raw: 0100000000000000 ff1100000103c3c0 ffd4000000162340 dead000000000003
raw: 0000000000000000 0000000080100010 00000001f5000000 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ff1100000588e180: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
 ff1100000588e200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ff1100000588e280: 00 fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
                      ^
 ff1100000588e300: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
 ff1100000588e380: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
==================================================================

---------------
thanks,
Kun Hu

Re: Bug: slab-out-of-bounds in snd_seq_oss_synth_sysex

[Kun Hu]

Hello,

> BUG: KASAN: slab-out-of-bounds in snd_seq_oss_synth_sysex+0x5d1/0x6c0 sound/core/seq/oss/seq_oss_synth.c:516

We further analyzed the issue at line 516 in ./sound/core/seq/oss/seq_oss_synth.c. 
The slab-out-of-bounds crash occurs in line 509, when sysex->len = 128. Specifically, the write operation to dest[0] accesses memory beyond the bounds of sysex->buf (128 byte).
To resolve this issue, we suggest adding 6 lines of code to validate the legality of the address write to sysex->buf before entering the loop:

if (sysex->len >= MAX_SYSEX_BUFLEN) { 
   sysex->len = 0; 
   sysex->skip = 1; 
   return -EINVAL;  /* Exit early if sysex->len is out of bounds */ 
}

If you fix this issue, please add the following tag to the commit:
Reported-by: Kun Hu <huk23@m.fudan.edu.cn>

—————
Thanks,
Kun Hu

> 2024年12月24日 19:16，Kun Hu <huk23@m.fudan.edu.cn> 写道：
> 
> Hello,
> 
> When using fuzzer tool to fuzz the latest Linux kernel, the following crash
> was triggered.
> 
> HEAD commit: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
> git tree: upstream
> Console output:https://drive.google.com/file/d/17oCyKDW_kNhSW5Bbvm23vnpD1eo0MHFi/view?usp=sharing
> Kernel config: https://drive.google.com/file/d/1RhT5dFTs6Vx1U71PbpenN7TPtnPoa3NI/view?usp=sharing
> C reproducer: https://drive.google.com/file/d/177HJht6a7-6F3YLudKb_d4kiPGd1VA_i/view?usp=sharing
> Syzlang reproducer: https://drive.google.com/file/d/1AuP5UGGc47rEXXPuvjmCKgJ3d0U1P84j/view?usp=sharing
> 
> 
> If you fix this issue, please add the following tag to the commit:
> Reported-by: Kun Hu <huk23@m.fudan.edu.cn>
> 
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in snd_seq_oss_synth_sysex+0x5d1/0x6c0 sound/core/seq/oss/seq_oss_synth.c:516
> Write of size 1 at addr ff1100000588e288 by task syz-executor411/824
> 
> CPU: 2 UID: 0 PID: 824 Comm: syz-executor411 Not tainted 6.13.0-rc3 #5
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
> Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:94 [inline]
> dump_stack_lvl+0x116/0x1b0 lib/dump_stack.c:120
> print_address_description mm/kasan/report.c:378 [inline]
> print_report+0xcf/0x5f0 mm/kasan/report.c:489
> kasan_report+0x93/0xc0 mm/kasan/report.c:602
> snd_seq_oss_synth_sysex+0x5d1/0x6c0 sound/core/seq/oss/seq_oss_synth.c:516
> snd_seq_oss_process_event+0x46a/0x2620 sound/core/seq/oss/seq_oss_event.c:61
> insert_queue sound/core/seq/oss/seq_oss_rw.c:167 [inline]
> snd_seq_oss_write+0x261/0x7f0 sound/core/seq/oss/seq_oss_rw.c:135
> odev_write+0x53/0xa0 sound/core/seq/oss/seq_oss.c:168
> vfs_write fs/read_write.c:677 [inline]
> vfs_write+0x2e3/0x10f0 fs/read_write.c:659
> ksys_write+0x122/0x240 fs/read_write.c:731
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xc3/0x1d0 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7f69b379994d
> Code: c3 e8 97 2a 00 00 0f 1f 80 00 00 00 00 f3 0f 1e fa 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 b8 ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007f69b3718d78 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> RAX: ffffffffffffffda RBX: 00007f69b382d2d0 RCX: 00007f69b379994d
> RDX: 0000000000000008 RSI: 0000000020000140 RDI: 0000000000000003
> RBP: 00007f69b382d2d8 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00007f69b382d2dc
> R13: 0000000020000140 R14: 00007f69b37fa008 R15: 000000000000000d
> </TASK>
> 
> Allocated by task 823:
> kasan_save_stack+0x24/0x50 mm/kasan/common.c:47
> kasan_save_track+0x14/0x30 mm/kasan/common.c:68
> poison_kmalloc_redzone mm/kasan/common.c:377 [inline]
> __kasan_kmalloc+0xaa/0xb0 mm/kasan/common.c:394
> kmalloc_noprof include/linux/slab.h:901 [inline]
> kzalloc_noprof include/linux/slab.h:1037 [inline]
> snd_seq_oss_synth_sysex+0x1d9/0x6c0 sound/core/seq/oss/seq_oss_synth.c:502
> snd_seq_oss_process_event+0x46a/0x2620 sound/core/seq/oss/seq_oss_event.c:61
> insert_queue sound/core/seq/oss/seq_oss_rw.c:167 [inline]
> snd_seq_oss_write+0x261/0x7f0 sound/core/seq/oss/seq_oss_rw.c:135
> odev_write+0x53/0xa0 sound/core/seq/oss/seq_oss.c:168
> vfs_write fs/read_write.c:677 [inline]
> vfs_write+0x2e3/0x10f0 fs/read_write.c:659
> ksys_write+0x122/0x240 fs/read_write.c:731
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xc3/0x1d0 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
> The buggy address belongs to the object at ff1100000588e200
> which belongs to the cache kmalloc-192 of size 192
> The buggy address is located 0 bytes to the right of
> allocated 136-byte region [ff1100000588e200, ff1100000588e288)
> 
> The buggy address belongs to the physical page:
> page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x588e
> anon flags: 0x100000000000000(node=0|zone=1)
> page_type: f5(slab)
> raw: 0100000000000000 ff1100000103c3c0 ffd4000000162340 dead000000000003
> raw: 0000000000000000 0000000080100010 00000001f5000000 0000000000000000
> page dumped because: kasan: bad access detected
> 
> Memory state around the buggy address:
> ff1100000588e180: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
> ff1100000588e200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> ff1100000588e280: 00 fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>                      ^
> ff1100000588e300: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ff1100000588e380: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
> ==================================================================
> 
> ---------------
> thanks,
> Kun Hu

Re: Bug: slab-out-of-bounds in snd_seq_oss_synth_sysex

[Kun Hu]

>> BUG: KASAN: slab-out-of-bounds in snd_seq_oss_synth_sysex+0x5d1/0x6c0 sound/core/seq/oss/seq_oss_synth.c:516

> 
> We further analyzed the issue at line 516 in ./sound/core/seq/oss/seq_oss_synth.c. 
> The slab-out-of-bounds crash occurs in line 509, when sysex->len = 128. Specifically, the write operation to dest[0] accesses memory beyond the bounds of sysex->buf (128 byte).
> To resolve this issue, we suggest adding 6 lines of code to validate the legality of the address write to sysex->buf before entering the loop:
> 
> if (sysex->len >= MAX_SYSEX_BUFLEN) { 
>   sysex->len = 0; 
>   sysex->skip = 1; 
>   return -EINVAL;  /* Exit early if sysex->len is out of bounds */ 
> }

If you need any more information, please don't hesitate to let me know.

—————
Best regards,
Kun Hu


> 2024年12月25日 13:37，Kun Hu <huk23@m.fudan.edu.cn> 写道：
> 
> Hello,
> 
>> BUG: KASAN: slab-out-of-bounds in snd_seq_oss_synth_sysex+0x5d1/0x6c0 sound/core/seq/oss/seq_oss_synth.c:516
> 
> We further analyzed the issue at line 516 in ./sound/core/seq/oss/seq_oss_synth.c. 
> The slab-out-of-bounds crash occurs in line 509, when sysex->len = 128. Specifically, the write operation to dest[0] accesses memory beyond the bounds of sysex->buf (128 byte).
> To resolve this issue, we suggest adding 6 lines of code to validate the legality of the address write to sysex->buf before entering the loop:
> 
> if (sysex->len >= MAX_SYSEX_BUFLEN) { 
>   sysex->len = 0; 
>   sysex->skip = 1; 
>   return -EINVAL;  /* Exit early if sysex->len is out of bounds */ 
> }
> 
> If you fix this issue, please add the following tag to the commit:
> Reported-by: Kun Hu <huk23@m.fudan.edu.cn>
> 
> —————
> Thanks,
> Kun Hu
> 
>> 2024年12月24日 19:16，Kun Hu <huk23@m.fudan.edu.cn> 写道：
>> 
>> Hello,
>> 
>> When using fuzzer tool to fuzz the latest Linux kernel, the following crash
>> was triggered.
>> 
>> HEAD commit: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
>> git tree: upstream
>> Console output:https://drive.google.com/file/d/17oCyKDW_kNhSW5Bbvm23vnpD1eo0MHFi/view?usp=sharing
>> Kernel config: https://drive.google.com/file/d/1RhT5dFTs6Vx1U71PbpenN7TPtnPoa3NI/view?usp=sharing
>> C reproducer: https://drive.google.com/file/d/177HJht6a7-6F3YLudKb_d4kiPGd1VA_i/view?usp=sharing
>> Syzlang reproducer: https://drive.google.com/file/d/1AuP5UGGc47rEXXPuvjmCKgJ3d0U1P84j/view?usp=sharing
>> 
>> 
>> If you fix this issue, please add the following tag to the commit:
>> Reported-by: Kun Hu <huk23@m.fudan.edu.cn>
>> 
>> ==================================================================
>> BUG: KASAN: slab-out-of-bounds in snd_seq_oss_synth_sysex+0x5d1/0x6c0 sound/core/seq/oss/seq_oss_synth.c:516
>> Write of size 1 at addr ff1100000588e288 by task syz-executor411/824
>> 
>> CPU: 2 UID: 0 PID: 824 Comm: syz-executor411 Not tainted 6.13.0-rc3 #5
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
>> Call Trace:
>> <TASK>
>> __dump_stack lib/dump_stack.c:94 [inline]
>> dump_stack_lvl+0x116/0x1b0 lib/dump_stack.c:120
>> print_address_description mm/kasan/report.c:378 [inline]
>> print_report+0xcf/0x5f0 mm/kasan/report.c:489
>> kasan_report+0x93/0xc0 mm/kasan/report.c:602
>> snd_seq_oss_synth_sysex+0x5d1/0x6c0 sound/core/seq/oss/seq_oss_synth.c:516
>> snd_seq_oss_process_event+0x46a/0x2620 sound/core/seq/oss/seq_oss_event.c:61
>> insert_queue sound/core/seq/oss/seq_oss_rw.c:167 [inline]
>> snd_seq_oss_write+0x261/0x7f0 sound/core/seq/oss/seq_oss_rw.c:135
>> odev_write+0x53/0xa0 sound/core/seq/oss/seq_oss.c:168
>> vfs_write fs/read_write.c:677 [inline]
>> vfs_write+0x2e3/0x10f0 fs/read_write.c:659
>> ksys_write+0x122/0x240 fs/read_write.c:731
>> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>> do_syscall_64+0xc3/0x1d0 arch/x86/entry/common.c:83
>> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>> RIP: 0033:0x7f69b379994d
>> Code: c3 e8 97 2a 00 00 0f 1f 80 00 00 00 00 f3 0f 1e fa 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 b8 ff ff ff f7 d8 64 89 01 48
>> RSP: 002b:00007f69b3718d78 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
>> RAX: ffffffffffffffda RBX: 00007f69b382d2d0 RCX: 00007f69b379994d
>> RDX: 0000000000000008 RSI: 0000000020000140 RDI: 0000000000000003
>> RBP: 00007f69b382d2d8 R08: 0000000000000000 R09: 0000000000000000
>> R10: 0000000000000000 R11: 0000000000000246 R12: 00007f69b382d2dc
>> R13: 0000000020000140 R14: 00007f69b37fa008 R15: 000000000000000d
>> </TASK>
>> 
>> Allocated by task 823:
>> kasan_save_stack+0x24/0x50 mm/kasan/common.c:47
>> kasan_save_track+0x14/0x30 mm/kasan/common.c:68
>> poison_kmalloc_redzone mm/kasan/common.c:377 [inline]
>> __kasan_kmalloc+0xaa/0xb0 mm/kasan/common.c:394
>> kmalloc_noprof include/linux/slab.h:901 [inline]
>> kzalloc_noprof include/linux/slab.h:1037 [inline]
>> snd_seq_oss_synth_sysex+0x1d9/0x6c0 sound/core/seq/oss/seq_oss_synth.c:502
>> snd_seq_oss_process_event+0x46a/0x2620 sound/core/seq/oss/seq_oss_event.c:61
>> insert_queue sound/core/seq/oss/seq_oss_rw.c:167 [inline]
>> snd_seq_oss_write+0x261/0x7f0 sound/core/seq/oss/seq_oss_rw.c:135
>> odev_write+0x53/0xa0 sound/core/seq/oss/seq_oss.c:168
>> vfs_write fs/read_write.c:677 [inline]
>> vfs_write+0x2e3/0x10f0 fs/read_write.c:659
>> ksys_write+0x122/0x240 fs/read_write.c:731
>> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>> do_syscall_64+0xc3/0x1d0 arch/x86/entry/common.c:83
>> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>> 
>> The buggy address belongs to the object at ff1100000588e200
>> which belongs to the cache kmalloc-192 of size 192
>> The buggy address is located 0 bytes to the right of
>> allocated 136-byte region [ff1100000588e200, ff1100000588e288)
>> 
>> The buggy address belongs to the physical page:
>> page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x588e
>> anon flags: 0x100000000000000(node=0|zone=1)
>> page_type: f5(slab)
>> raw: 0100000000000000 ff1100000103c3c0 ffd4000000162340 dead000000000003
>> raw: 0000000000000000 0000000080100010 00000001f5000000 0000000000000000
>> page dumped because: kasan: bad access detected
>> 
>> Memory state around the buggy address:
>> ff1100000588e180: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
>> ff1100000588e200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>> ff1100000588e280: 00 fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>>                     ^
>> ff1100000588e300: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>> ff1100000588e380: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
>> ==================================================================
>> 
>> ---------------
>> thanks,
>> Kun Hu
> 
> 

Re: Bug: slab-out-of-bounds in snd_seq_oss_synth_sysex

[Kun Hu]

> 
> 
>> 2024年12月25日 13:37，Kun Hu <huk23@m.fudan.edu.cn> 写道：
>> 
>> Hello,
>> 
>>> BUG: KASAN: slab-out-of-bounds in snd_seq_oss_synth_sysex+0x5d1/0x6c0 sound/core/seq/oss/seq_oss_synth.c:516
>> 
>> We further analyzed the issue at line 516 in ./sound/core/seq/oss/seq_oss_synth.c. 
>> The slab-out-of-bounds crash occurs in line 509, when sysex->len = 128. Specifically, the write operation to dest[0] accesses memory beyond the bounds of sysex->buf (128 byte).
>> To resolve this issue, we suggest adding 6 lines of code to validate the legality of the address write to sysex->buf before entering the loop:
>> 
>> if (sysex->len >= MAX_SYSEX_BUFLEN) { 
>>  sysex->len = 0; 
>>  sysex->skip = 1; 
>>  return -EINVAL;  /* Exit early if sysex->len is out of bounds */ 
>> }
>> 
>> If you fix this issue, please add the following tag to the commit:
>> Reported-by: Kun Hu <huk23@m.fudan.edu.cn>
>> 
>> —————
>> Thanks,
>> Kun Hu
>> 
>>> 2024年12月24日 19:16，Kun Hu <huk23@m.fudan.edu.cn> 写道：
>>> 
>>> Hello,
>>> 
>>> When using fuzzer tool to fuzz the latest Linux kernel, the following crash
>>> was triggered.
>>> 
>>> HEAD commit: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
>>> git tree: upstream
>>> Console output:https://drive.google.com/file/d/17oCyKDW_kNhSW5Bbvm23vnpD1eo0MHFi/view?usp=sharing
>>> Kernel config: https://drive.google.com/file/d/1RhT5dFTs6Vx1U71PbpenN7TPtnPoa3NI/view?usp=sharing
>>> C reproducer: https://drive.google.com/file/d/177HJht6a7-6F3YLudKb_d4kiPGd1VA_i/view?usp=sharing
>>> Syzlang reproducer: https://drive.google.com/file/d/1AuP5UGGc47rEXXPuvjmCKgJ3d0U1P84j/view?usp=sharing
>>> 
>>> 
>>> If you fix this issue, please add the following tag to the commit:
>>> Reported-by: Kun Hu <huk23@m.fudan.edu.cn>
>>> 
>>> ==================================================================
>>> BUG: KASAN: slab-out-of-bounds in snd_seq_oss_synth_sysex+0x5d1/0x6c0 sound/core/seq/oss/seq_oss_synth.c:516
>>> Write of size 1 at addr ff1100000588e288 by task syz-executor411/824
>>> 
>>> CPU: 2 UID: 0 PID: 824 Comm: syz-executor411 Not tainted 6.13.0-rc3 #5
>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
>>> Call Trace:
>>> <TASK>
>>> __dump_stack lib/dump_stack.c:94 [inline]
>>> dump_stack_lvl+0x116/0x1b0 lib/dump_stack.c:120
>>> print_address_description mm/kasan/report.c:378 [inline]
>>> print_report+0xcf/0x5f0 mm/kasan/report.c:489
>>> kasan_report+0x93/0xc0 mm/kasan/report.c:602
>>> snd_seq_oss_synth_sysex+0x5d1/0x6c0 sound/core/seq/oss/seq_oss_synth.c:516
>>> snd_seq_oss_process_event+0x46a/0x2620 sound/core/seq/oss/seq_oss_event.c:61
>>> insert_queue sound/core/seq/oss/seq_oss_rw.c:167 [inline]
>>> snd_seq_oss_write+0x261/0x7f0 sound/core/seq/oss/seq_oss_rw.c:135
>>> odev_write+0x53/0xa0 sound/core/seq/oss/seq_oss.c:168
>>> vfs_write fs/read_write.c:677 [inline]
>>> vfs_write+0x2e3/0x10f0 fs/read_write.c:659
>>> ksys_write+0x122/0x240 fs/read_write.c:731
>>> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>>> do_syscall_64+0xc3/0x1d0 arch/x86/entry/common.c:83
>>> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>> 
> 

Hello, 

Is this issue being considered and is it possible that the value of sysex->len in line 509 of the snd_seq_oss_synth_sysex function could exceed 127 and thus be out of bounds?

———
Best Regards,
Kun Hu

Re: Bug: slab-out-of-bounds in snd_seq_oss_synth_sysex

[Takashi Iwai]

On Sat, 28 Dec 2024 09:07:16 +0100,
Kun Hu wrote:
> 
> > 
> > 
> >> 2024年12月25日 13:37，Kun Hu <huk23@m.fudan.edu.cn> 写道：
> >> 
> >> Hello,
> >> 
> >>> BUG: KASAN: slab-out-of-bounds in snd_seq_oss_synth_sysex+0x5d1/0x6c0 sound/core/seq/oss/seq_oss_synth.c:516
> >> 
> >> We further analyzed the issue at line 516 in ./sound/core/seq/oss/seq_oss_synth.c. 
> >> The slab-out-of-bounds crash occurs in line 509, when sysex->len = 128. Specifically, the write operation to dest[0] accesses memory beyond the bounds of sysex->buf (128 byte).
> >> To resolve this issue, we suggest adding 6 lines of code to validate the legality of the address write to sysex->buf before entering the loop:
> >> 
> >> if (sysex->len >= MAX_SYSEX_BUFLEN) { 
> >>  sysex->len = 0; 
> >>  sysex->skip = 1; 
> >>  return -EINVAL;  /* Exit early if sysex->len is out of bounds */ 
> >> }
> >> 
> >> If you fix this issue, please add the following tag to the commit:
> >> Reported-by: Kun Hu <huk23@m.fudan.edu.cn>
> >> 
> >> ―――――
> >> Thanks,
> >> Kun Hu
> >> 
> >>> 2024年12月24日 19:16，Kun Hu <huk23@m.fudan.edu.cn> 写道：
> >>> 
> >>> Hello,
> >>> 
> >>> When using fuzzer tool to fuzz the latest Linux kernel, the following crash
> >>> was triggered.
> >>> 
> >>> HEAD commit: 78d4f34e2115b517bcbfe7ec0d018bbbb6f9b0b8
> >>> git tree: upstream
> >>> Console output:https://drive.google.com/file/d/17oCyKDW_kNhSW5Bbvm23vnpD1eo0MHFi/view?usp=sharing
> >>> Kernel config: https://drive.google.com/file/d/1RhT5dFTs6Vx1U71PbpenN7TPtnPoa3NI/view?usp=sharing
> >>> C reproducer: https://drive.google.com/file/d/177HJht6a7-6F3YLudKb_d4kiPGd1VA_i/view?usp=sharing
> >>> Syzlang reproducer: https://drive.google.com/file/d/1AuP5UGGc47rEXXPuvjmCKgJ3d0U1P84j/view?usp=sharing
> >>> 
> >>> 
> >>> If you fix this issue, please add the following tag to the commit:
> >>> Reported-by: Kun Hu <huk23@m.fudan.edu.cn>
> >>> 
> >>> ==================================================================
> >>> BUG: KASAN: slab-out-of-bounds in snd_seq_oss_synth_sysex+0x5d1/0x6c0 sound/core/seq/oss/seq_oss_synth.c:516
> >>> Write of size 1 at addr ff1100000588e288 by task syz-executor411/824
> >>> 
> >>> CPU: 2 UID: 0 PID: 824 Comm: syz-executor411 Not tainted 6.13.0-rc3 #5
> >>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014
> >>> Call Trace:
> >>> <TASK>
> >>> __dump_stack lib/dump_stack.c:94 [inline]
> >>> dump_stack_lvl+0x116/0x1b0 lib/dump_stack.c:120
> >>> print_address_description mm/kasan/report.c:378 [inline]
> >>> print_report+0xcf/0x5f0 mm/kasan/report.c:489
> >>> kasan_report+0x93/0xc0 mm/kasan/report.c:602
> >>> snd_seq_oss_synth_sysex+0x5d1/0x6c0 sound/core/seq/oss/seq_oss_synth.c:516
> >>> snd_seq_oss_process_event+0x46a/0x2620 sound/core/seq/oss/seq_oss_event.c:61
> >>> insert_queue sound/core/seq/oss/seq_oss_rw.c:167 [inline]
> >>> snd_seq_oss_write+0x261/0x7f0 sound/core/seq/oss/seq_oss_rw.c:135
> >>> odev_write+0x53/0xa0 sound/core/seq/oss/seq_oss.c:168
> >>> vfs_write fs/read_write.c:677 [inline]
> >>> vfs_write+0x2e3/0x10f0 fs/read_write.c:659
> >>> ksys_write+0x122/0x240 fs/read_write.c:731
> >>> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> >>> do_syscall_64+0xc3/0x1d0 arch/x86/entry/common.c:83
> >>> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> >> 
> > 
> 
> Hello, 
> 
> Is this issue being considered and is it possible that the value of sysex->len in line 509 of the snd_seq_oss_synth_sysex function could exceed 127 and thus be out of bounds?

Sorry for the late reaction, as I've been (still) off since the last
week.  Will check in details later.

But, through a quick glance, it's likely the racy access, and your
suggested fix won't suffice, I'm afraid.


thanks,

Takashi

Re: Bug: slab-out-of-bounds in snd_seq_oss_synth_sysex

[Kun Hu]

> Sorry for the late reaction, as I've been (still) off since the last
> week.  Will check in details later.
> 
> But, through a quick glance, it's likely the racy access, and your
> suggested fix won't suffice, I'm afraid.


So sorry to have disturbed your rest. Let me know if you need any more info.

Best Regards,

Kun Hu

Re: Bug: slab-out-of-bounds in snd_seq_oss_synth_sysex

[Takashi Iwai]

On Wed, 25 Dec 2024 06:37:47 +0100,
Kun Hu wrote:
> 
> Hello,
> 
> > BUG: KASAN: slab-out-of-bounds in snd_seq_oss_synth_sysex+0x5d1/0x6c0 sound/core/seq/oss/seq_oss_synth.c:516
> 
> We further analyzed the issue at line 516 in ./sound/core/seq/oss/seq_oss_synth.c. 
> The slab-out-of-bounds crash occurs in line 509, when sysex->len = 128. Specifically, the write operation to dest[0] accesses memory beyond the bounds of sysex->buf (128 byte).
> To resolve this issue, we suggest adding 6 lines of code to validate the legality of the address write to sysex->buf before entering the loop:
> 
> if (sysex->len >= MAX_SYSEX_BUFLEN) { 
>    sysex->len = 0; 
>    sysex->skip = 1; 
>    return -EINVAL;  /* Exit early if sysex->len is out of bounds */ 
> }

The only place incrementing sysex->len is that loop and it has already
the bounce check which resets sysex->len to 0.  That is, the likely
reason of the buffer overflow is rather the racy calls of this
function.

If so, a potential fix would be rather to protect the concurrency,
e.g. a simplest form is something like below.
Could you check whether it addresses your problem?


thanks,

Takashi

-- 8< --
--- a/sound/core/seq/oss/seq_oss_synth.c
+++ b/sound/core/seq/oss/seq_oss_synth.c
@@ -66,6 +66,7 @@ static struct seq_oss_synth midi_synth_dev = {
 };
 
 static DEFINE_SPINLOCK(register_lock);
+static DEFINE_MUTEX(sysex_mutex);
 
 /*
  * prototypes
@@ -497,6 +498,7 @@ snd_seq_oss_synth_sysex(struct seq_oss_devinfo *dp, int dev, unsigned char *buf,
 	if (!info)
 		return -ENXIO;
 
+	guard(mutex)(&sysex_mutex);
 	sysex = info->sysex;
 	if (sysex == NULL) {
 		sysex = kzalloc(sizeof(*sysex), GFP_KERNEL);

Re: Bug: slab-out-of-bounds in snd_seq_oss_synth_sysex

[Kun Hu]

> 
> The only place incrementing sysex->len is that loop and it has already
> the bounce check which resets sysex->len to 0.  That is, the likely
> reason of the buffer overflow is rather the racy calls of this
> function.
> 
> If so, a potential fix would be rather to protect the concurrency,
> e.g. a simplest form is something like below.
> Could you check whether it addresses your problem?
> 
> 
> thanks,
> 
> Takashi
> 
> -- 8< --
> --- a/sound/core/seq/oss/seq_oss_synth.c
> +++ b/sound/core/seq/oss/seq_oss_synth.c
> @@ -66,6 +66,7 @@ static struct seq_oss_synth midi_synth_dev = {
> };
> 
> static DEFINE_SPINLOCK(register_lock);
> +static DEFINE_MUTEX(sysex_mutex);
> 
> /*
>  * prototypes
> @@ -497,6 +498,7 @@ snd_seq_oss_synth_sysex(struct seq_oss_devinfo *dp, int dev, unsigned char *buf,
> if (!info)
> return -ENXIO;
> 
> + guard(mutex)(&sysex_mutex);
> sysex = info->sysex;
> if (sysex == NULL) {
> sysex = kzalloc(sizeof(*sysex), GFP_KERNEL);

Hi,

I did a lot of reproduction testing and it was very easy to reproduce before the lock was added, after the lock was added the error was no longer reproducible.

Thanks,
Kun Hu

Re: Bug: slab-out-of-bounds in snd_seq_oss_synth_sysex

[Takashi Iwai]

On Mon, 30 Dec 2024 10:47:08 +0100,
Kun Hu wrote:
> 
> 
>     The only place incrementing sysex->len is that loop and it has already
>     the bounce check which resets sysex->len to 0.  That is, the likely
>     reason of the buffer overflow is rather the racy calls of this
>     function.
>    
>     If so, a potential fix would be rather to protect the concurrency,
>     e.g. a simplest form is something like below.
>     Could you check whether it addresses your problem?
> 
>     thanks,
>    
>     Takashi
>    
>     -- 8< --
>     --- a/sound/core/seq/oss/seq_oss_synth.c
>     +++ b/sound/core/seq/oss/seq_oss_synth.c
>     @@ -66,6 +66,7 @@ static struct seq_oss_synth midi_synth_dev = {
>     };
>    
>     static DEFINE_SPINLOCK(register_lock);
>     +static DEFINE_MUTEX(sysex_mutex);
>    
>     /*
>      * prototypes
>     @@ -497,6 +498,7 @@ snd_seq_oss_synth_sysex(struct seq_oss_devinfo *dp,
>     int dev, unsigned char *buf,
>     if (!info)
>     return -ENXIO;
>    
>     + guard(mutex)(&sysex_mutex);
>     sysex = info->sysex;
>     if (sysex == NULL) {
>     sysex = kzalloc(sizeof(*sysex), GFP_KERNEL);
> 
> Hi,
> 
> I did a lot of reproduction testing and it was very easy to reproduce before
> the lock was added, after the lock was added the error was no longer
> reproducible.

Good to hear.  Then I'm going to submit this as a band-aid fix; this
is simple and easy to backport to older stable releases, too.

Meanwhile, we may have a different fix for the long term.  Essentially
the sysex handling in OSS layer is unnecessarily complex, and we can
send a packet immediately at each time instead of combining them.

Could you try the patch below instead of the previous one?


thanks,

Takashi

-- 8< --
diff --git a/sound/core/seq/oss/seq_oss_device.h b/sound/core/seq/oss/seq_oss_device.h
index 98dd20b42976..c0b1cce127a3 100644
--- a/sound/core/seq/oss/seq_oss_device.h
+++ b/sound/core/seq/oss/seq_oss_device.h
@@ -55,7 +55,6 @@ struct seq_oss_chinfo {
 struct seq_oss_synthinfo {
 	struct snd_seq_oss_arg arg;
 	struct seq_oss_chinfo *ch;
-	struct seq_oss_synth_sysex *sysex;
 	int nr_voices;
 	int opened;
 	int is_midi;
diff --git a/sound/core/seq/oss/seq_oss_synth.c b/sound/core/seq/oss/seq_oss_synth.c
index e3394919daa0..02a90c960992 100644
--- a/sound/core/seq/oss/seq_oss_synth.c
+++ b/sound/core/seq/oss/seq_oss_synth.c
@@ -26,13 +26,6 @@
  * definition of synth info records
  */
 
-/* sysex buffer */
-struct seq_oss_synth_sysex {
-	int len;
-	int skip;
-	unsigned char buf[MAX_SYSEX_BUFLEN];
-};
-
 /* synth info */
 struct seq_oss_synth {
 	int seq_device;
@@ -318,8 +311,6 @@ snd_seq_oss_synth_cleanup(struct seq_oss_devinfo *dp)
 			}
 			snd_use_lock_free(&rec->use_lock);
 		}
-		kfree(info->sysex);
-		info->sysex = NULL;
 		kfree(info->ch);
 		info->ch = NULL;
 	}
@@ -395,8 +386,6 @@ snd_seq_oss_synth_reset(struct seq_oss_devinfo *dp, int dev)
 	info = get_synthinfo_nospec(dp, dev);
 	if (!info || !info->opened)
 		return;
-	if (info->sysex)
-		info->sysex->len = 0; /* reset sysex */
 	reset_channels(info);
 	if (info->is_midi) {
 		if (midi_synth_dev.opened <= 0)
@@ -408,8 +397,6 @@ snd_seq_oss_synth_reset(struct seq_oss_devinfo *dp, int dev)
 					  dp->file_mode) < 0) {
 			midi_synth_dev.opened--;
 			info->opened = 0;
-			kfree(info->sysex);
-			info->sysex = NULL;
 			kfree(info->ch);
 			info->ch = NULL;
 		}
@@ -482,63 +469,26 @@ snd_seq_oss_synth_info(struct seq_oss_devinfo *dp, int dev)
 
 /*
  * receive OSS 6 byte sysex packet:
- * the full sysex message will be sent if it reaches to the end of data
- * (0xff).
+ * the event is filled and prepared for sending immediately
+ * (i.e. sysex messages are fragmented)
  */
 int
 snd_seq_oss_synth_sysex(struct seq_oss_devinfo *dp, int dev, unsigned char *buf, struct snd_seq_event *ev)
 {
-	int i, send;
-	unsigned char *dest;
-	struct seq_oss_synth_sysex *sysex;
-	struct seq_oss_synthinfo *info;
+	unsigned char *p;
+	int len = 6;
 
-	info = snd_seq_oss_synth_info(dp, dev);
-	if (!info)
-		return -ENXIO;
+	p = memchr(buf, 0xff, 6);
+	if (p)
+		len = p - buf + 1;
 
-	sysex = info->sysex;
-	if (sysex == NULL) {
-		sysex = kzalloc(sizeof(*sysex), GFP_KERNEL);
-		if (sysex == NULL)
-			return -ENOMEM;
-		info->sysex = sysex;
-	}
-
-	send = 0;
-	dest = sysex->buf + sysex->len;
-	/* copy 6 byte packet to the buffer */
-	for (i = 0; i < 6; i++) {
-		if (buf[i] == 0xff) {
-			send = 1;
-			break;
-		}
-		dest[i] = buf[i];
-		sysex->len++;
-		if (sysex->len >= MAX_SYSEX_BUFLEN) {
-			sysex->len = 0;
-			sysex->skip = 1;
-			break;
-		}
-	}
-
-	if (sysex->len && send) {
-		if (sysex->skip) {
-			sysex->skip = 0;
-			sysex->len = 0;
-			return -EINVAL; /* skip */
-		}
-		/* copy the data to event record and send it */
-		ev->flags = SNDRV_SEQ_EVENT_LENGTH_VARIABLE;
-		if (snd_seq_oss_synth_addr(dp, dev, ev))
-			return -EINVAL;
-		ev->data.ext.len = sysex->len;
-		ev->data.ext.ptr = sysex->buf;
-		sysex->len = 0;
-		return 0;
-	}
-
-	return -EINVAL; /* skip */
+	/* copy the data to event record and send it */
+	if (snd_seq_oss_synth_addr(dp, dev, ev))
+		return -EINVAL;
+	ev->flags = SNDRV_SEQ_EVENT_LENGTH_VARIABLE;
+	ev->data.ext.len = len;
+	ev->data.ext.ptr = buf;
+	return 0;
 }
 
 /*

Re: Bug: slab-out-of-bounds in snd_seq_oss_synth_sysex

[Kun Hu]

> 
> Good to hear.  Then I'm going to submit this as a band-aid fix; this
> is simple and easy to backport to older stable releases, too.
> 
> Meanwhile, we may have a different fix for the long term.  Essentially
> the sysex handling in OSS layer is unnecessarily complex, and we can
> send a packet immediately at each time instead of combining them.
> 
> Could you try the patch below instead of the previous one?
> 
> 
> thanks,
> 
> Takashi
> 
> -- 8< --
> diff --git a/sound/core/seq/oss/seq_oss_device.h b/sound/core/seq/oss/seq_oss_device.h
> index 98dd20b42976..c0b1cce127a3 100644
> --- a/sound/core/seq/oss/seq_oss_device.h
> +++ b/sound/core/seq/oss/seq_oss_device.h
> @@ -55,7 +55,6 @@ struct seq_oss_chinfo {
> struct seq_oss_synthinfo {
> struct snd_seq_oss_arg arg;
> struct seq_oss_chinfo *ch;
> - struct seq_oss_synth_sysex *sysex;
> int nr_voices;
> int opened;
> int is_midi;
> diff --git a/sound/core/seq/oss/seq_oss_synth.c b/sound/core/seq/oss/seq_oss_synth.c
> index e3394919daa0..02a90c960992 100644
> --- a/sound/core/seq/oss/seq_oss_synth.c
> +++ b/sound/core/seq/oss/seq_oss_synth.c
> @@ -26,13 +26,6 @@
>  * definition of synth info records
>  */
> 
> -/* sysex buffer */
> -struct seq_oss_synth_sysex {
> - int len;
> - int skip;
> - unsigned char buf[MAX_SYSEX_BUFLEN];
> -};
> -
> /* synth info */
> struct seq_oss_synth {
> int seq_device;
> @@ -318,8 +311,6 @@ snd_seq_oss_synth_cleanup(struct seq_oss_devinfo *dp)
> }
> snd_use_lock_free(&rec->use_lock);
> }
> - kfree(info->sysex);
> - info->sysex = NULL;
> kfree(info->ch);
> info->ch = NULL;
> }
> @@ -395,8 +386,6 @@ snd_seq_oss_synth_reset(struct seq_oss_devinfo *dp, int dev)
> info = get_synthinfo_nospec(dp, dev);
> if (!info || !info->opened)
> return;
> - if (info->sysex)
> - info->sysex->len = 0; /* reset sysex */
> reset_channels(info);
> if (info->is_midi) {
> if (midi_synth_dev.opened <= 0)
> @@ -408,8 +397,6 @@ snd_seq_oss_synth_reset(struct seq_oss_devinfo *dp, int dev)
>   dp->file_mode) < 0) {
> midi_synth_dev.opened--;
> info->opened = 0;
> - kfree(info->sysex);
> - info->sysex = NULL;
> kfree(info->ch);
> info->ch = NULL;
> }
> @@ -482,63 +469,26 @@ snd_seq_oss_synth_info(struct seq_oss_devinfo *dp, int dev)
> 
> /*
>  * receive OSS 6 byte sysex packet:
> - * the full sysex message will be sent if it reaches to the end of data
> - * (0xff).
> + * the event is filled and prepared for sending immediately
> + * (i.e. sysex messages are fragmented)
>  */
> int
> snd_seq_oss_synth_sysex(struct seq_oss_devinfo *dp, int dev, unsigned char *buf, struct snd_seq_event *ev)
> {
> - int i, send;
> - unsigned char *dest;
> - struct seq_oss_synth_sysex *sysex;
> - struct seq_oss_synthinfo *info;
> + unsigned char *p;
> + int len = 6;
> 
> - info = snd_seq_oss_synth_info(dp, dev);
> - if (!info)
> - return -ENXIO;
> + p = memchr(buf, 0xff, 6);
> + if (p)
> + len = p - buf + 1;
> 
> - sysex = info->sysex;
> - if (sysex == NULL) {
> - sysex = kzalloc(sizeof(*sysex), GFP_KERNEL);
> - if (sysex == NULL)
> - return -ENOMEM;
> - info->sysex = sysex;
> - }
> -
> - send = 0;
> - dest = sysex->buf + sysex->len;
> - /* copy 6 byte packet to the buffer */
> - for (i = 0; i < 6; i++) {
> - if (buf[i] == 0xff) {
> - send = 1;
> - break;
> - }
> - dest[i] = buf[i];
> - sysex->len++;
> - if (sysex->len >= MAX_SYSEX_BUFLEN) {
> - sysex->len = 0;
> - sysex->skip = 1;
> - break;
> - }
> - }
> -
> - if (sysex->len && send) {
> - if (sysex->skip) {
> - sysex->skip = 0;
> - sysex->len = 0;
> - return -EINVAL; /* skip */
> - }
> - /* copy the data to event record and send it */
> - ev->flags = SNDRV_SEQ_EVENT_LENGTH_VARIABLE;
> - if (snd_seq_oss_synth_addr(dp, dev, ev))
> - return -EINVAL;
> - ev->data.ext.len = sysex->len;
> - ev->data.ext.ptr = sysex->buf;
> - sysex->len = 0;
> - return 0;
> - }
> -
> - return -EINVAL; /* skip */
> + /* copy the data to event record and send it */
> + if (snd_seq_oss_synth_addr(dp, dev, ev))
> + return -EINVAL;
> + ev->flags = SNDRV_SEQ_EVENT_LENGTH_VARIABLE;
> + ev->data.ext.len = len;
> + ev->data.ext.ptr = buf;
> + return 0;
> }
> 
> /*

Hi,

Regarding your changes, I’ve conducted multiple rounds of race condition testing using the same method. The original issue, caused by competition for shared resources, could consistently reproduce an out-of-bounds error in just a few attempts. The temporary band-aid fix and the latest changes have been tested at least 10,000 times, and no issues have occurred so far.

——
Thanks,
Kun Hu

Re: Bug: slab-out-of-bounds in snd_seq_oss_synth_sysex

[Takashi Iwai]

On Mon, 30 Dec 2024 14:10:24 +0100,
Kun Hu wrote:
> 
> 
> > 
> > Good to hear.  Then I'm going to submit this as a band-aid fix; this
> > is simple and easy to backport to older stable releases, too.
> > 
> > Meanwhile, we may have a different fix for the long term.  Essentially
> > the sysex handling in OSS layer is unnecessarily complex, and we can
> > send a packet immediately at each time instead of combining them.
> > 
> > Could you try the patch below instead of the previous one?
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> > -- 8< --
> > diff --git a/sound/core/seq/oss/seq_oss_device.h b/sound/core/seq/oss/seq_oss_device.h
> > index 98dd20b42976..c0b1cce127a3 100644
> > --- a/sound/core/seq/oss/seq_oss_device.h
> > +++ b/sound/core/seq/oss/seq_oss_device.h
> > @@ -55,7 +55,6 @@ struct seq_oss_chinfo {
> > struct seq_oss_synthinfo {
> > struct snd_seq_oss_arg arg;
> > struct seq_oss_chinfo *ch;
> > - struct seq_oss_synth_sysex *sysex;
> > int nr_voices;
> > int opened;
> > int is_midi;
> > diff --git a/sound/core/seq/oss/seq_oss_synth.c b/sound/core/seq/oss/seq_oss_synth.c
> > index e3394919daa0..02a90c960992 100644
> > --- a/sound/core/seq/oss/seq_oss_synth.c
> > +++ b/sound/core/seq/oss/seq_oss_synth.c
> > @@ -26,13 +26,6 @@
> >  * definition of synth info records
> >  */
> > 
> > -/* sysex buffer */
> > -struct seq_oss_synth_sysex {
> > - int len;
> > - int skip;
> > - unsigned char buf[MAX_SYSEX_BUFLEN];
> > -};
> > -
> > /* synth info */
> > struct seq_oss_synth {
> > int seq_device;
> > @@ -318,8 +311,6 @@ snd_seq_oss_synth_cleanup(struct seq_oss_devinfo *dp)
> > }
> > snd_use_lock_free(&rec->use_lock);
> > }
> > - kfree(info->sysex);
> > - info->sysex = NULL;
> > kfree(info->ch);
> > info->ch = NULL;
> > }
> > @@ -395,8 +386,6 @@ snd_seq_oss_synth_reset(struct seq_oss_devinfo *dp, int dev)
> > info = get_synthinfo_nospec(dp, dev);
> > if (!info || !info->opened)
> > return;
> > - if (info->sysex)
> > - info->sysex->len = 0; /* reset sysex */
> > reset_channels(info);
> > if (info->is_midi) {
> > if (midi_synth_dev.opened <= 0)
> > @@ -408,8 +397,6 @@ snd_seq_oss_synth_reset(struct seq_oss_devinfo *dp, int dev)
> >   dp->file_mode) < 0) {
> > midi_synth_dev.opened--;
> > info->opened = 0;
> > - kfree(info->sysex);
> > - info->sysex = NULL;
> > kfree(info->ch);
> > info->ch = NULL;
> > }
> > @@ -482,63 +469,26 @@ snd_seq_oss_synth_info(struct seq_oss_devinfo *dp, int dev)
> > 
> > /*
> >  * receive OSS 6 byte sysex packet:
> > - * the full sysex message will be sent if it reaches to the end of data
> > - * (0xff).
> > + * the event is filled and prepared for sending immediately
> > + * (i.e. sysex messages are fragmented)
> >  */
> > int
> > snd_seq_oss_synth_sysex(struct seq_oss_devinfo *dp, int dev, unsigned char *buf, struct snd_seq_event *ev)
> > {
> > - int i, send;
> > - unsigned char *dest;
> > - struct seq_oss_synth_sysex *sysex;
> > - struct seq_oss_synthinfo *info;
> > + unsigned char *p;
> > + int len = 6;
> > 
> > - info = snd_seq_oss_synth_info(dp, dev);
> > - if (!info)
> > - return -ENXIO;
> > + p = memchr(buf, 0xff, 6);
> > + if (p)
> > + len = p - buf + 1;
> > 
> > - sysex = info->sysex;
> > - if (sysex == NULL) {
> > - sysex = kzalloc(sizeof(*sysex), GFP_KERNEL);
> > - if (sysex == NULL)
> > - return -ENOMEM;
> > - info->sysex = sysex;
> > - }
> > -
> > - send = 0;
> > - dest = sysex->buf + sysex->len;
> > - /* copy 6 byte packet to the buffer */
> > - for (i = 0; i < 6; i++) {
> > - if (buf[i] == 0xff) {
> > - send = 1;
> > - break;
> > - }
> > - dest[i] = buf[i];
> > - sysex->len++;
> > - if (sysex->len >= MAX_SYSEX_BUFLEN) {
> > - sysex->len = 0;
> > - sysex->skip = 1;
> > - break;
> > - }
> > - }
> > -
> > - if (sysex->len && send) {
> > - if (sysex->skip) {
> > - sysex->skip = 0;
> > - sysex->len = 0;
> > - return -EINVAL; /* skip */
> > - }
> > - /* copy the data to event record and send it */
> > - ev->flags = SNDRV_SEQ_EVENT_LENGTH_VARIABLE;
> > - if (snd_seq_oss_synth_addr(dp, dev, ev))
> > - return -EINVAL;
> > - ev->data.ext.len = sysex->len;
> > - ev->data.ext.ptr = sysex->buf;
> > - sysex->len = 0;
> > - return 0;
> > - }
> > -
> > - return -EINVAL; /* skip */
> > + /* copy the data to event record and send it */
> > + if (snd_seq_oss_synth_addr(dp, dev, ev))
> > + return -EINVAL;
> > + ev->flags = SNDRV_SEQ_EVENT_LENGTH_VARIABLE;
> > + ev->data.ext.len = len;
> > + ev->data.ext.ptr = buf;
> > + return 0;
> > }
> > 
> > /*
> 
> Hi,
> 
> Regarding your changes, I’ve conducted multiple rounds of race condition testing using the same method. The original issue, caused by competition for shared resources, could consistently reproduce an out-of-bounds error in just a few attempts. The temporary band-aid fix and the latest changes have been tested at least 10,000 times, and no issues have occurred so far.

OK, that's convincing enough.
I'm going to submit the proper patch.  It won't be queued for 6.13 but
for 6.14, though.


thanks,

Takashi

Re: Bug: slab-out-of-bounds in snd_seq_oss_synth_sysex

[Kun Hu]

> 
> Hi,
> 
> Regarding your changes, I’ve conducted multiple rounds of race condition testing using the same method. The original issue, caused by competition for shared resources, could consistently reproduce an out-of-bounds error in just a few attempts. The temporary band-aid fix and the latest changes have been tested at least 10,000 times, and no issues have occurred so far.
> 
> ——
> Thanks,
> Kun Hu


Okay, Thanks for your help.

——
Best regards,
Kun Hu