Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: KASAN: null-ptr-deref Write in io_uring_cancel_sqpoll
To:     Palash Oswal <oswalpalash@gmail.com>,
        Dmitry Vyukov <dvyukov@google.com>
Cc:     Jens Axboe <axboe@kernel.dk>, io-uring@vger.kernel.org,
        LKML <linux-kernel@vger.kernel.org>,
        syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
        syzbot+be51ca5a4d97f017cd50@syzkaller.appspotmail.com
References: <00000000000022ebeb05bc39f582@google.com>
 <e939af11-7ce8-46af-8c76-651add0ae56bn@googlegroups.com>
 <CACT4Y+aPRCZcLvkuWgK=A_rR0PqdEAM+xssWU4N7hNRSm9=mSA@mail.gmail.com>
 <CAGyP=7fBRPc+qH9UvhGhid9j-B2PeYhQ4bbde_Vg72Mnx9z75Q@mail.gmail.com>
From:   Pavel Begunkov <asml.silence@gmail.com>
Message-ID: <dba3f0a9-cb5d-a162-b696-864295259581@gmail.com>
Date:   Tue, 27 Apr 2021 09:37:33 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.9.1
MIME-Version: 1.0
In-Reply-To: <CAGyP=7fBRPc+qH9UvhGhid9j-B2PeYhQ4bbde_Vg72Mnx9z75Q@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 4/27/21 8:05 AM, Palash Oswal wrote:
>> +kernel lists and syzbot email
>> (almost nobody is reading syzkaller-bugs@ itself)
> Thanks Dmitry. I used "reply-all" in the google groups UI, and I
> didn't check the cc list before hitting send :/
> 
> I have made progress on this bug. I applied a diff (to print some
> debug values) on the v5.12 fs/io_uring.c code and got a fairly
> consistent reproducer on a non-kvm based qemu VM (had to slow down the
> rate of syscalls processed for this to trigger early).
> 
> My initial speculation was very wrong. And the real issue seems to be
> that current->io_uring is unset when io_uring_cancel_sqpoll is called.
> 
> Adding the c reproducer here:
> #define _GNU_SOURCE
> 
> #include <endian.h>
> #include <signal.h>
> #include <stdint.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <sys/mman.h>
> #include <sys/stat.h>
> #include <sys/syscall.h>
> #include <sys/types.h>
> #include <unistd.h>
> #include <time.h>
> #include <sys/wait.h>
> 
> #define sys_io_uring_setup 425
> #define WAIT_FLAGS __WALL
> 
> static unsigned long long procid;
> 
> static void sleep_ms(uint64_t ms)
> {
>     usleep(ms * 1000);
> }
> 
> 
> static void kill_and_wait(int pid, int* status)
> {
>     kill(-pid, SIGKILL);
>     kill(pid, SIGKILL);
>     for (int i = 0; i < 100; i++) {
>         if (waitpid(-1, status, WNOHANG | __WALL) == pid)
>             return;
>         usleep(1000);
>     }
>     while (waitpid(-1, status, __WALL) != pid) {
>     }
> }
> 
> static void execute_one(void) {
>     *(uint32_t*)0x20000084 = 0x850e;
>     *(uint32_t*)0x20000088 = 2;
>     *(uint32_t*)0x2000008c = 2;
>     *(uint32_t*)0x20000090 = 0x1b4;
>     *(uint32_t*)0x20000098 = -1;
>     memset((void*)0x2000009c, 0, 12);
>     syscall(sys_io_uring_setup, 0x329b, 0x20000080);
> }
> 
> static uint64_t current_time_ms(void)
> {
>     struct timespec ts;
>     if (clock_gettime(CLOCK_MONOTONIC, &ts))
>         exit(1);
>     return (uint64_t)ts.tv_sec * 1000 + (uint64_t)ts.tv_nsec / 1000000;
> }
> 
> 
> static void loop(void)
> {
>     int iter = 0;
>     for (;; iter++) {
>         int pid = fork();
>         if (pid < 0)
>             exit(1);
>         if (pid == 0) {
>             execute_one();
>             exit(0);
>         }
>         int status = 0;
>         uint64_t start = current_time_ms();
>         for (;;) {
>             if (waitpid(-1, &status, WNOHANG | WAIT_FLAGS) == pid)
>                 break;
>             sleep_ms(1);
>             kill_and_wait(pid, &status);
>             break;
>         }
>     }
> }
> 
> 
> 
> 
> int main(void)
> {
>     syscall(__NR_mmap, 0x20000000ul, 0x1000000ul, 7ul, 0x32ul, -1, 0ul);
>     for (procid = 0; procid < 8; procid++) {
>         if (fork() == 0) {
>             loop();
>         }
>     }
>     sleep(1000000);
>     return 0;
> }
> Some debug console logs:
> [   58.455071] ctx is 39386608
> [   58.455415] io_uring_cancel_sqpoll called with ctx :39386080
> [   58.455913] ctx->sq_data is 00000000c9f086d5
> [   58.456146] current is 000000005e7bf8a0
> [   58.456346] current->io_uring is :0000000000000000
> [   58.457244] ==================================================================
> [   58.457663] BUG: KASAN: null-ptr-deref in io_uring_cancel_sqpoll+0x341/0x4b0
> [   58.458214] Write of size 4 at addr 0000000000000060 by task
> iou-sqp-1857/1860
> [   58.458699]
> [   58.459061] CPU: 1 PID: 1860 Comm: iou-sqp-1857 Not tainted 5.12.0+ #83
> [   58.459522] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 1.14.0-1 04/01/2014
> [   58.460065] Call Trace:
> [   58.460402]  dump_stack+0xe9/0x168
> [   58.460749]  ? io_uring_cancel_sqpoll+0x341/0x4b0
> [   58.460846]  __kasan_report+0x166/0x1c0
> [   58.460846]  ? io_uring_cancel_sqpoll+0x341/0x4b0
> [   58.460846]  kasan_report+0x4f/0x70
> [   58.460846]  kasan_check_range+0x2f3/0x340
> [   58.460846]  __kasan_check_write+0x14/0x20
> [   58.460846]  io_uring_cancel_sqpoll+0x341/0x4b0
> [   58.460846]  ? io_sq_thread_unpark+0xd0/0xd0
> [   58.460846]  ? init_wait_entry+0xe0/0xe0
> [   58.460846]  io_sq_thread+0x1a0d/0x1c50
> [   58.460846]  ? io_rsrc_put_work+0x380/0x380
> [   58.460846]  ? init_wait_entry+0xe0/0xe0
> [   58.460846]  ? _raw_spin_lock_irq+0xa5/0x180
> [   58.460846]  ? _raw_spin_lock_irqsave+0x190/0x190
> [   58.460846]  ? calculate_sigpending+0x6b/0xa0
> [   58.460846]  ? io_rsrc_put_work+0x380/0x380
> [   58.460846]  ret_from_fork+0x22/0x30
> 
> I'm going to look further into why this is happening.

io_sq_offload_create() {
    ...
    ret = io_uring_alloc_task_context(tsk, ctx);
    wake_up_new_task(tsk);
    if (ret)
        goto err;
}

Shouldn't happen unless offload create has failed. Just add
a return in *cancel_sqpoll() for this case. It's failing
so no requests has been submitted and no cancellation is needed.

-- 
Pavel Begunkov