[syzbot] UBSAN: array-index-out-of-bounds in io_submit_sqes

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: 3b46e4e44180 Add linux-next specific files for 20220531
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=16e151f5f00000
kernel config: https://syzkaller.appspot.com/x/.config?x=ccb8d66fc9489ef
dashboard link: https://syzkaller.appspot.com/bug?extid=b6c9b65b6753d333d833
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2

Unfortunately, I don't have any reproducer for this issue yet.

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

================================================================================
================================================================================
UBSAN: array-index-out-of-bounds in fs/io_uring.c:8860:19
index 75 is out of range for type 'io_op_def [47]'
CPU: 0 PID: 10377 Comm: syz-executor.4 Not tainted 5.18.0-next-20220531-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
ubsan_epilogue+0xb/0x50 lib/ubsan.c:151
__ubsan_handle_out_of_bounds.cold+0x62/0x6c lib/ubsan.c:283
io_init_req fs/io_uring.c:8860 [inline]
io_submit_sqe fs/io_uring.c:8987 [inline]
io_submit_sqes+0x6f0e/0x8020 fs/io_uring.c:9143
__do_sys_io_uring_enter+0x1112/0x2300 fs/io_uring.c:12077
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x35/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x46/0xb0
RIP: 0033:0x7fd28ac89109
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 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fd28be25168 EFLAGS: 00000246 ORIG_RAX: 00000000000001aa
RAX: ffffffffffffffda RBX: 00007fd28ad9bf60 RCX: 00007fd28ac89109
RDX: 0000000000000000 RSI: 00000000000001b9 RDI: 0000000000000003
RBP: 00007fd28ace308d R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007ffe7683a70f R14: 00007fd28be25300 R15: 0000000000022000
</TASK>
================================================================================


---
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 syzk...@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

[Jens Axboe]

On 5/31/22 1:55 AM, syzbot wrote:
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: 3b46e4e44180 Add linux-next specific files for 20220531
> git tree: linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=16e151f5f00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=ccb8d66fc9489ef
> dashboard link: https://syzkaller.appspot.com/bug?extid=b6c9b65b6753d333d833
> compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
>
> Unfortunately, I don't have any reproducer for this issue yet.
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+b6c9b6...@syzkaller.appspotmail.com
>
> ================================================================================
> ================================================================================
> UBSAN: array-index-out-of-bounds in fs/io_uring.c:8860:19
> index 75 is out of range for type 'io_op_def [47]'

'def' is just set here, it's not actually used after 'opcode' has been
verified.

--
Jens Axboe

[Dmitry Vyukov]

An interesting thing about C is that now the compiler is within its
rights to actually remove the check that is supposed to validate the
index because indexing io_op_defs[opcode] implies that opcode is
already within bounds, otherwise the program already has undefined
behavior, so removing the check is that case is also OK ;)

[Jens Axboe]

I did fix this up as I think it's just a bug waiting to happen anyway.

--
Jens Axboe

[Hao Xu]

Maybe we can move it to be below the opcode check to comfort UBSAN.

[Jens Axboe]

Yeah that's what I did, just rebased it to get rid of it:

https://git.kernel.dk/cgit/linux-block/commit/?h=io_uring-5.19&id=fcde59feb1affb6d56aecadc3868df4631480da5

--
Jens Axboe

[Dmitry Vyukov]

If you are rebasing it, please add the following tag so that the bug
is closed later:

Tested-by: syzbot+b6c9b6...@syzkaller.appspotmail.com

[Jens Axboe]

Sorry, missed that, would be a bit confusing? 5.20 branch is rebased
on top of that too. Can we just do:

#syz fix: io_uring: add io_op_defs 'def' pointer in req init and issue

?

--
Jens Axboe

[Dmitry Vyukov]

Why confusing? It tested it, no?

> 5.20 branch is rebased
> on top of that too. Can we just do:
>
> #syz fix: io_uring: add io_op_defs 'def' pointer in req init and issue
>
> ?

In most cases it will work. However, there is no way to distinguish
unfixed and fixed versions of the patch based on the title.
So if the unfixed version manages to reach all syzbot builds, it will
close the bug at that point. And then can start reporting duplicates
since the bug is still present. But practically unlikely to happen.
The tag allows to distinguish unfixed and fixed versions of the patch,
so it will work reliably w/o possible duplicates.

[Jens Axboe]

Usually I'd use that tag if it's a separate commit that fixes an issue,
and someone (or a bot) has tested it. I think we both agree that the
change will fix it, but not really tested at that point. Or maybe it is
now :)

>
>> 5.20 branch is rebased
>> on top of that too. Can we just do:
>>
>> #syz fix: io_uring: add io_op_defs 'def' pointer in req init and issue
>>
>> ?
>
> In most cases it will work. However, there is no way to distinguish
> unfixed and fixed versions of the patch based on the title.
> So if the unfixed version manages to reach all syzbot builds, it will
> close the bug at that point. And then can start reporting duplicates
> since the bug is still present. But practically unlikely to happen.
> The tag allows to distinguish unfixed and fixed versions of the patch,
> so it will work reliably w/o possible duplicates.

Gotcha. Usually I don't rebase anyway, but easier in this case.

--
Jens Axboe