KASAN: use-after-free Write in io_wq_worker_running

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: f4d51dff Linux 5.9-rc4
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=13686dcd900000
kernel config: https://syzkaller.appspot.com/x/.config?x=8f5c353182ed6199
dashboard link: https://syzkaller.appspot.com/bug?extid=45fa0a195b941764e0f0
compiler: clang version 10.0.0 (https://github.com/llvm/llvm-project/ c2443155a0fb245c8f17f2c1c72b6ea391e86e81)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1378ceed900000

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

==================================================================
BUG: KASAN: use-after-free in instrument_atomic_write include/linux/instrumented.h:71 [inline]
BUG: KASAN: use-after-free in atomic_inc include/asm-generic/atomic-instrumented.h:240 [inline]
BUG: KASAN: use-after-free in io_wqe_inc_running fs/io-wq.c:301 [inline]
BUG: KASAN: use-after-free in io_wq_worker_running+0xb4/0x100 fs/io-wq.c:613
Write of size 4 at addr ffff88821aaa388c by task io_wqe_worker-0/8276

CPU: 0 PID: 8276 Comm: io_wqe_worker-0 Not tainted 5.9.0-rc4-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1d6/0x29e lib/dump_stack.c:118
print_address_description+0x66/0x620 mm/kasan/report.c:383
__kasan_report mm/kasan/report.c:513 [inline]
kasan_report+0x132/0x1d0 mm/kasan/report.c:530
check_memory_region_inline mm/kasan/generic.c:183 [inline]
check_memory_region+0x2b5/0x2f0 mm/kasan/generic.c:192
instrument_atomic_write include/linux/instrumented.h:71 [inline]
atomic_inc include/asm-generic/atomic-instrumented.h:240 [inline]
io_wqe_inc_running fs/io-wq.c:301 [inline]
io_wq_worker_running+0xb4/0x100 fs/io-wq.c:613
schedule_timeout+0x15c/0x250 kernel/time/timer.c:1879
io_wqe_worker+0x60b/0x810 fs/io-wq.c:580
kthread+0x37e/0x3a0 drivers/block/aoe/aoecmd.c:1234
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294

Allocated by task 8273:
kasan_save_stack mm/kasan/common.c:48 [inline]
kasan_set_track mm/kasan/common.c:56 [inline]
__kasan_kmalloc+0x100/0x130 mm/kasan/common.c:461
kmem_cache_alloc_node_trace+0x1f7/0x2a0 mm/slab.c:3594
kmalloc_node include/linux/slab.h:572 [inline]
kzalloc_node include/linux/slab.h:677 [inline]
io_wq_create+0x295/0x880 fs/io-wq.c:1064
io_init_wq_offload fs/io_uring.c:7432 [inline]
io_sq_offload_start fs/io_uring.c:7504 [inline]
io_uring_create fs/io_uring.c:8625 [inline]
io_uring_setup fs/io_uring.c:8694 [inline]
__do_sys_io_uring_setup fs/io_uring.c:8700 [inline]
__se_sys_io_uring_setup+0x18ed/0x2a00 fs/io_uring.c:8697
do_syscall_64+0x31/0x70 arch/x86/entry/common.c:46
entry_SYSCALL_64_after_hwframe+0x44/0xa9

Freed by task 8175:
kasan_save_stack mm/kasan/common.c:48 [inline]
kasan_set_track+0x3d/0x70 mm/kasan/common.c:56
kasan_set_free_info+0x17/0x30 mm/kasan/generic.c:355
__kasan_slab_free+0xdd/0x110 mm/kasan/common.c:422
__cache_free mm/slab.c:3418 [inline]
kfree+0x113/0x200 mm/slab.c:3756
__io_wq_destroy fs/io-wq.c:1138 [inline]
io_wq_destroy+0x470/0x510 fs/io-wq.c:1146
io_finish_async fs/io_uring.c:6836 [inline]
io_ring_ctx_free fs/io_uring.c:7870 [inline]
io_ring_exit_work+0x195/0x520 fs/io_uring.c:7954
process_one_work+0x789/0xfc0 kernel/workqueue.c:2269
worker_thread+0xaa4/0x1460 kernel/workqueue.c:2415
kthread+0x37e/0x3a0 drivers/block/aoe/aoecmd.c:1234
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294

The buggy address belongs to the object at ffff88821aaa3800
which belongs to the cache kmalloc-1k of size 1024
The buggy address is located 140 bytes inside of
1024-byte region [ffff88821aaa3800, ffff88821aaa3c00)
The buggy address belongs to the page:
page:00000000be451134 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x21aaa3
flags: 0x57ffe0000000200(slab)
raw: 057ffe0000000200 ffffea00086a2148 ffffea0008536b08 ffff8880aa440700
raw: 0000000000000000 ffff88821aaa3000 0000000100000002 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
ffff88821aaa3780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff88821aaa3800: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff88821aaa3880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88821aaa3900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88821aaa3980: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================


---
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.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches

[syzbot]

syzbot has found a reproducer for the following issue on:

HEAD commit: 581cb3a2 Merge tag 'f2fs-for-5.9-rc5' of git://git.kernel...
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=10cc47a5900000
kernel config: https://syzkaller.appspot.com/x/.config?x=a9075b36a6ae26c9
dashboard link: https://syzkaller.appspot.com/bug?extid=45fa0a195b941764e0f0
compiler: gcc (GCC) 10.1.0-syz 20200507
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=12e933f9900000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1554acdd900000

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

==================================================================
BUG: KASAN: use-after-free in instrument_atomic_write include/linux/instrumented.h:71 [inline]
BUG: KASAN: use-after-free in atomic_inc include/asm-generic/atomic-instrumented.h:240 [inline]
BUG: KASAN: use-after-free in io_wqe_inc_running fs/io-wq.c:301 [inline]

BUG: KASAN: use-after-free in io_wq_worker_running+0xde/0x110 fs/io-wq.c:613
Write of size 4 at addr ffff8882183db08c by task io_wqe_worker-0/7771

CPU: 0 PID: 7771 Comm: io_wqe_worker-0 Not tainted 5.9.0-rc4-syzkaller #0

Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]

dump_stack+0x198/0x1fd lib/dump_stack.c:118
print_address_description.constprop.0.cold+0xae/0x497 mm/kasan/report.c:383
__kasan_report mm/kasan/report.c:513 [inline]
kasan_report.cold+0x1f/0x37 mm/kasan/report.c:530
check_memory_region_inline mm/kasan/generic.c:186 [inline]
check_memory_region+0x13d/0x180 mm/kasan/generic.c:192

instrument_atomic_write include/linux/instrumented.h:71 [inline]
atomic_inc include/asm-generic/atomic-instrumented.h:240 [inline]
io_wqe_inc_running fs/io-wq.c:301 [inline]

io_wq_worker_running+0xde/0x110 fs/io-wq.c:613
schedule_timeout+0x148/0x250 kernel/time/timer.c:1879
io_wqe_worker+0x517/0x10e0 fs/io-wq.c:580
kthread+0x3b5/0x4a0 kernel/kthread.c:292
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294

Allocated by task 7768:
kasan_save_stack+0x1b/0x40 mm/kasan/common.c:48
kasan_set_track mm/kasan/common.c:56 [inline]
__kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:461
kmem_cache_alloc_node_trace+0x17b/0x3f0 mm/slab.c:3594

kmalloc_node include/linux/slab.h:572 [inline]
kzalloc_node include/linux/slab.h:677 [inline]

io_wq_create+0x57b/0xa10 fs/io-wq.c:1064

io_init_wq_offload fs/io_uring.c:7432 [inline]
io_sq_offload_start fs/io_uring.c:7504 [inline]
io_uring_create fs/io_uring.c:8625 [inline]

io_uring_setup+0x1836/0x28e0 fs/io_uring.c:8694
do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
entry_SYSCALL_64_after_hwframe+0x44/0xa9

Freed by task 21:
kasan_save_stack+0x1b/0x40 mm/kasan/common.c:48
kasan_set_track+0x1c/0x30 mm/kasan/common.c:56
kasan_set_free_info+0x1b/0x30 mm/kasan/generic.c:355
__kasan_slab_free+0xd8/0x120 mm/kasan/common.c:422
__cache_free mm/slab.c:3418 [inline]
kfree+0x10e/0x2b0 mm/slab.c:3756
__io_wq_destroy fs/io-wq.c:1138 [inline]
io_wq_destroy+0x2af/0x460 fs/io-wq.c:1146

io_finish_async fs/io_uring.c:6836 [inline]
io_ring_ctx_free fs/io_uring.c:7870 [inline]

io_ring_exit_work+0x1e4/0x6d0 fs/io_uring.c:7954
process_one_work+0x94c/0x1670 kernel/workqueue.c:2269
worker_thread+0x64c/0x1120 kernel/workqueue.c:2415
kthread+0x3b5/0x4a0 kernel/kthread.c:292
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294

The buggy address belongs to the object at ffff8882183db000

which belongs to the cache kmalloc-1k of size 1024
The buggy address is located 140 bytes inside of

1024-byte region [ffff8882183db000, ffff8882183db400)

The buggy address belongs to the page:

page:000000009bada22b refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x2183db
flags: 0x57ffe0000000200(slab)
raw: 057ffe0000000200 ffffea0008604c48 ffffea00086a8648 ffff8880aa040700
raw: 0000000000000000 ffff8882183db000 0000000100000002 0000000000000000

page dumped because: kasan: bad access detected

Memory state around the buggy address:

ffff8882183daf80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
ffff8882183db000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff8882183db080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8882183db100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8882183db180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================

[Hillf Danton]

Mon, 07 Sep 2020 15:45:20 -0700

Looks like the tracking of workers makes the

wait_for_completion(&wq->done);

in __io_wq_destroy() fail to work, which likely accounts also for
the uaf Reported-by: syzbot+956ef5...@syzkaller.appspotmail.com

Hillf

[Hillf Danton]

Mon, 07 Sep 2020 15:45:20 -0700

Fix breaks into two parts:

1, No worker will be created if work queue is being destroyed.

--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -637,9 +637,12 @@ void io_wq_worker_sleeping(struct task_s

static bool create_io_worker(struct io_wq *wq, struct io_wqe *wqe, int index)
{
- struct io_wqe_acct *acct =&wqe->acct[index];
+ struct io_wqe_acct *acct = &wqe->acct[index];
struct io_worker *worker;

+ if (test_bit(IO_WQ_BIT_EXIT, &wq->state))
+ return false;
+
worker = kzalloc_node(sizeof(*worker), GFP_KERNEL, wqe->node);
if (!worker)
return false;
--

2, Make work queue's refcount independent of the tracking of workers and
keep it balanced for every worker that comes and goes. Note the manager
thread now is no longer special in terms of holding a grab to the refcount.

--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -231,11 +231,6 @@ static void io_worker_exit(struct io_wor
nr_workers = wqe->acct[IO_WQ_ACCT_BOUND].nr_workers +
wqe->acct[IO_WQ_ACCT_UNBOUND].nr_workers;
raw_spin_unlock_irq(&wqe->lock);
-
- /* all workers gone, wq exit can proceed */
- if (!nr_workers && refcount_dec_and_test(&wqe->wq->refs))
- complete(&wqe->wq->done);
-
kfree_rcu(worker, rcu);
}

@@ -594,6 +589,8 @@ loop:
}

io_worker_exit(worker);
+ if (refcount_dec_and_test(&wq->refs))
+ complete(&wq->done);
return 0;
}

@@ -670,6 +667,7 @@ static bool create_io_worker(struct io_w
acct->nr_workers++;
raw_spin_unlock_irq(&wqe->lock);

+ refcount_inc(&wq->refs);
if (index == IO_WQ_ACCT_UNBOUND)
atomic_inc(&wq->user->processes);

@@ -694,22 +692,17 @@ static inline bool io_wqe_need_worker(st
static int io_wq_manager(void *data)
{
struct io_wq *wq = data;
- int workers_to_create = num_possible_nodes();
int node;

/* create fixed workers */
- refcount_set(&wq->refs, workers_to_create);
+ refcount_set(&wq->refs, 1);
for_each_node(node) {
if (!node_online(node))
continue;
if (!create_io_worker(wq, wq->wqes[node], IO_WQ_ACCT_BOUND))
goto err;
- workers_to_create--;
}

- while (workers_to_create--)
- refcount_dec(&wq->refs);
-
complete(&wq->done);

while (!kthread_should_stop()) {
@@ -740,14 +733,14 @@ static int io_wq_manager(void *data)

if (current->task_works)
task_work_run();
-
+out:
+ if (refcount_dec_and_test(&wq->refs))
+ complete(&wq->done);
return 0;
err:
set_bit(IO_WQ_BIT_ERROR, &wq->state);
set_bit(IO_WQ_BIT_EXIT, &wq->state);
- if (refcount_sub_and_test(workers_to_create, &wq->refs))
- complete(&wq->done);
- return 0;
+ goto out;
}

static bool io_wq_can_queue(struct io_wqe *wqe, struct io_wqe_acct *acct,
--

[Hillf Danton]

Fri, 11 Sep 2020 10:28:18 -0700

The uaf reported can be down to the comment below,

/* all workers gone, wq exit can proceed */

if (!nr_workers && refcount_dec_and_test(&wqe->wq->refs))

complete(&wqe->wq->done);

because there might be multiple wqes in a wq and we would wait for
every worker in every wqe to go home before releasing the wq's resources.

> CPU: 0 PID: 7771 Comm: io_wqe_worker-0 Not tainted 5.9.0-rc4-syzkaller #0

btw, the worker name like the above one does not help much in this report
wrt wqe and id. It should better look like io_wqe_wroker-unid-id IMO if we
can borrow something from workqueue, where unid == unbound node id and
id == the index of the worker ever created on nid.

Hillf