Re: [syzbot] [io-uring?] WARNING in __io_uring_free

Re: [syzbot] [io-uring?] WARNING in __io_uring_free

[Jann Horn]

+Matthew for xarray

On Tue, Nov 19, 2024 at 5:38 AM syzbot
<syzbot+cc36d44ec9f368e443d3@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit:    cfaaa7d010d1 Merge tag 'net-6.12-rc8' of git://git.kernel...
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=13005cc0580000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=d2aeec8c0b2e420c
> dashboard link: https://syzkaller.appspot.com/bug?extid=cc36d44ec9f368e443d3
> compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
>
> Unfortunately, I don't have any reproducer for this issue yet.
>
> Downloadable assets:
> disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/7feb34a89c2a/non_bootable_disk-cfaaa7d0.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/63eae0d3e67f/vmlinux-cfaaa7d0.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/6495d9e4ddee/bzImage-cfaaa7d0.xz
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+cc36d44ec9f368e443d3@syzkaller.appspotmail.com
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 16 at io_uring/tctx.c:51 __io_uring_free+0xfa/0x140 io_uring/tctx.c:51

This warning is a check for WARN_ON_ONCE(!xa_empty(&tctx->xa)); and as
Jens pointed out, this was triggered after error injection caused a
memory allocation inside xa_store() to fail.

Is there maybe an issue where xa_store() can fail midway through while
allocating memory for the xarray, so that xa_empty() is no longer true
even though there is nothing in the xarray? (And if yes, is that
working as intended?)

> Modules linked in:
> CPU: 0 UID: 0 PID: 16 Comm: ksoftirqd/0 Not tainted 6.12.0-rc7-syzkaller-00125-gcfaaa7d010d1 #0
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
> RIP: 0010:__io_uring_free+0xfa/0x140 io_uring/tctx.c:51
> Code: 80 7c 25 00 00 74 08 4c 89 f7 e8 a1 8a 49 fd 49 c7 06 00 00 00 00 5b 41 5c 41 5d 41 5e 41 5f c3 cc cc cc cc e8 37 ad df fc 90 <0f> 0b 90 e9 6a ff ff ff e8 29 ad df fc 90 0f 0b 90 eb 84 e8 1e ad
> RSP: 0018:ffffc900004279b8 EFLAGS: 00010246
> RAX: ffffffff84b53cd9 RBX: ffff88804fc3b8e0 RCX: ffff88801b7e8000
> RDX: 0000000000000100 RSI: 0000000000000000 RDI: ffff88801f058000
> RBP: 0000000000000001 R08: ffffffff8154d881 R09: 1ffff11003e0b005
> R10: dffffc0000000000 R11: ffffed1003e0b006 R12: dffffc0000000000
> R13: 1ffff11003e0b120 R14: ffff88801f058900 R15: ffff88804fc3b800
> FS:  0000000000000000(0000) GS:ffff88801fc00000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00005594393ad338 CR3: 000000000e734000 CR4: 0000000000352ef0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
>  <TASK>
>  io_uring_free include/linux/io_uring.h:31 [inline]
>  __put_task_struct+0xd5/0x290 kernel/fork.c:975
>  put_task_struct include/linux/sched/task.h:144 [inline]
>  delayed_put_task_struct+0x125/0x300 kernel/exit.c:228
>  rcu_do_batch kernel/rcu/tree.c:2567 [inline]
>  rcu_core+0xaaa/0x17a0 kernel/rcu/tree.c:2823
>  handle_softirqs+0x2c5/0x980 kernel/softirq.c:554
>  run_ksoftirqd+0xca/0x130 kernel/softirq.c:927
>  smpboot_thread_fn+0x544/0xa30 kernel/smpboot.c:164
>  kthread+0x2f0/0x390 kernel/kthread.c:389
>  ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
>  </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 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 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
>

Re: [syzbot] [io-uring?] WARNING in __io_uring_free

[Matthew Wilcox]

On Fri, Nov 29, 2024 at 12:30:35AM +0100, Jann Horn wrote:
> > ------------[ cut here ]------------
> > WARNING: CPU: 0 PID: 16 at io_uring/tctx.c:51 __io_uring_free+0xfa/0x140 io_uring/tctx.c:51
> 
> This warning is a check for WARN_ON_ONCE(!xa_empty(&tctx->xa)); and as
> Jens pointed out, this was triggered after error injection caused a
> memory allocation inside xa_store() to fail.
> 
> Is there maybe an issue where xa_store() can fail midway through while
> allocating memory for the xarray, so that xa_empty() is no longer true
> even though there is nothing in the xarray? (And if yes, is that
> working as intended?)

Yes, that's a known possibility.  We have similar problems when people
use error injection with mapping->i_pages.  The effort to fix it seems
disproportionate to the severity of the problem.

Re: [syzbot] [io-uring?] WARNING in __io_uring_free

[Jens Axboe]

On 11/28/24 4:57 PM, Matthew Wilcox wrote:
> On Fri, Nov 29, 2024 at 12:30:35AM +0100, Jann Horn wrote:
>>> ------------[ cut here ]------------
>>> WARNING: CPU: 0 PID: 16 at io_uring/tctx.c:51 __io_uring_free+0xfa/0x140 io_uring/tctx.c:51
>>
>> This warning is a check for WARN_ON_ONCE(!xa_empty(&tctx->xa)); and as
>> Jens pointed out, this was triggered after error injection caused a
>> memory allocation inside xa_store() to fail.
>>
>> Is there maybe an issue where xa_store() can fail midway through while
>> allocating memory for the xarray, so that xa_empty() is no longer true
>> even though there is nothing in the xarray? (And if yes, is that
>> working as intended?)

Heh, I had the exact same thought when I originally looked at this
issue. I did code inspection on the io_uring side and tried with error
injection, but could not trigger it. Hence the io_uring side looks fine,
so must be lower down.

> Yes, that's a known possibility.  We have similar problems when people
> use error injection with mapping->i_pages.  The effort to fix it seems
> disproportionate to the severity of the problem.

Doesn't seem like a big deal, particularly when you essentially need
fault injection to trigger it. As long as the xa_empty() is the only
false positive. I wonder if I should just change the io_uring side to do
something ala:

xa_for_each(&tctx->xa, index, node) {
	WARN_ON_ONCE(1);
	break;
}

rather than the xa_empty() warn on. That should get rid of it on my side
at least.

-- 
Jens Axboe

Re: [syzbot] [io-uring?] WARNING in __io_uring_free

[Fedor Pchelkin]

Matthew Wilcox wrote:
> On Fri, Nov 29, 2024 at 12:30:35AM +0100, Jann Horn wrote:
> > > ------------[ cut here ]------------
> > > WARNING: CPU: 0 PID: 16 at io_uring/tctx.c:51 __io_uring_free+0xfa/0x140 io_uring/tctx.c:51
> > 
> > This warning is a check for WARN_ON_ONCE(!xa_empty(&tctx->xa)); and as
> > Jens pointed out, this was triggered after error injection caused a
> > memory allocation inside xa_store() to fail.
> > 
> > Is there maybe an issue where xa_store() can fail midway through while
> > allocating memory for the xarray, so that xa_empty() is no longer true
> > even though there is nothing in the xarray? (And if yes, is that
> > working as intended?)
> 
> Yes, that's a known possibility.  We have similar problems when people
> use error injection with mapping->i_pages.  The effort to fix it seems
> disproportionate to the severity of the problem.

Found this discussion while investigating memory leak in radix_tree_insert [1].
That report has a similar cause - a fault injection in the innards of
radix_tree (say, xarray) allocating loop, then the absence of release of
already allocated internal xarray memory afterall.

I wonder whether just the plain usage of xa_destroy() should be considered
a fix for these kinds of failures. Are there any pitfalls? xa_destroy() is
claimed to cleanup the internal xarray memory.

Judging by ca6484cd308a ("io_uring: no need to call xa_destroy() on empty
xarray"), seems some pitfalls do exist but still..

Would be glad to have a look into the previous discussions of this problem
if they exist - in case I'm raising the questions that were already
answered. Thanks!

P.S. there is no variant of xa_destroy() for radix tree. I think nobody
noticed this since it may have an effect only on these types of bugs
triggered by fault injection. If you think adding it overall makes sense
then I'd try to prepare a patch.

[1]: https://syzkaller.appspot.com/bug?extid=006987d1be3586e13555