Re: [syzbot] [kvm?] [net?] [virt?] BUG: sleeping function called from invalid context in vhost_get_avail_idx

[Kohei Enju <kohei@enjuk.jp>]

On 04/20 15:09, syzbot wrote:
> Hello,
> 
> syzbot found the following issue on:
> 
> HEAD commit:    8541d8f725c6 Merge tag 'mtd/for-7.1' of git://git.kernel.o..
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=136454ce580000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=7e54da1916e8d11f
> dashboard link: https://syzkaller.appspot.com/bug?extid=6985cb8e543ea90ba8ee
> compiler:       gcc (Debian 14.2.0-19) 14.2.0, GNU ld (GNU Binutils for Debian) 2.44
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=15d264ce580000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=143ec1ba580000
> 
> Downloadable assets:
> disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/d900f083ada3/non_bootable_disk-8541d8f7.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/22dfea2c37c2/vmlinux-8541d8f7.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/e2f93ad68fe3/bzImage-8541d8f7.xz
> 
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+6985cb8e543ea90ba8ee@syzkaller.appspotmail.com
> 
> BUG: sleeping function called from invalid context at drivers/vhost/vhost.c:1527
> in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 6110, name: vhost-6109
> preempt_count: 1, expected: 0
> RCU nest depth: 0, expected: 0
> 2 locks held by vhost-6109/6110:
>  #0: ffff888055624cb0 (&vq->mutex/1){+.+.}-{4:4}, at: handle_tx+0x2d/0x160 drivers/vhost/net.c:971
>  #1: ffff888055620248 (&vq->mutex){+.+.}-{4:4}, at: vhost_net_busy_poll+0x9c/0x730 drivers/vhost/net.c:554
> Preemption disabled at:
> [<ffffffff88f1a006>] vhost_net_busy_poll+0x1c6/0x730 drivers/vhost/net.c:563

I think the blamed commit may be commit 030881372460 ("vhost_net: basic
polling support"), since it introduced preempt_{disable,enable}() around
the busy-poll loop, which calls a sleepable function inside the loop.

Also, from the changelog of the series,

https://lore.kernel.org/netdev/1448435489-5949-4-git-send-email-jasowang@redhat.com/T/#u

  Changes from RFC V1:
  ...
  - Disable preemption during busy looping to make sure local_clock() was
    correctly used.

So my understanding is that preempt_disable() was introduced to keep
local_clock() based timeout accounting on a single CPU, rather than as a
requirement of busy polling itself.

If my understanding is correct, migrate_disable() is sufficient here
instead of preempt_disable(), avoiding sleepable accesses from a
preempt-disabled context.

#syz test

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 80965181920c..c6536cad9c4f 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -560,7 +560,7 @@ static void vhost_net_busy_poll(struct vhost_net *net,
        busyloop_timeout = poll_rx ? rvq->busyloop_timeout:
                                     tvq->busyloop_timeout;

-       preempt_disable();
+       migrate_disable();
        endtime = busy_clock() + busyloop_timeout;

        while (vhost_can_busy_poll(endtime)) {
@@ -577,7 +577,7 @@ static void vhost_net_busy_poll(struct vhost_net *net,
                cpu_relax();
        }

-       preempt_enable();
+       migrate_enable();

        if (poll_rx || sock_has_rx_data(sock))
                vhost_net_busy_poll_try_queue(net, vq);


> CPU: 0 UID: 0 PID: 6110 Comm: vhost-6109 Not tainted syzkaller #0 PREEMPT(full) 
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> Call Trace:
>  <TASK>
>  __dump_stack lib/dump_stack.c:94 [inline]
>  dump_stack_lvl+0x100/0x190 lib/dump_stack.c:120
>  __might_resched.cold+0x1ec/0x232 kernel/sched/core.c:9162
>  __might_fault+0x8b/0x140 mm/memory.c:7322
>  vhost_get_avail_idx+0x31c/0x4f0 drivers/vhost/vhost.c:1527
>  vhost_vq_avail_empty drivers/vhost/vhost.c:3206 [inline]
>  vhost_vq_avail_empty+0xa9/0xe0 drivers/vhost/vhost.c:3199
>  vhost_net_busy_poll+0x297/0x730 drivers/vhost/net.c:574
>  vhost_net_tx_get_vq_desc drivers/vhost/net.c:610 [inline]
>  get_tx_bufs.constprop.0+0x338/0x600 drivers/vhost/net.c:650
>  handle_tx_copy+0x28c/0x12e0 drivers/vhost/net.c:778
>  handle_tx+0x139/0x160 drivers/vhost/net.c:985
>  vhost_run_work_list+0x183/0x220 drivers/vhost/vhost.c:454
>  vhost_task_fn+0x156/0x430 kernel/vhost_task.c:49
>  ret_from_fork+0x72b/0xd50 arch/x86/kernel/process.c:158
>  ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
>  </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 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