Jens Axboe <axboe@xxxxxxxxx> writes:
> If we specify a valid CQ ring address but an invalid SQ ring address,
> we'll correctly spot this and free the allocated pages and clear them
> to NULL. However, we don't clear the ring page count, and hence will
> attempt to free the pages again. We've already cleared the address of
> the page array when freeing them, but we don't check for that. This
> causes the following crash:
>
> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> Oops [#1]
> Modules linked in:
> CPU: 0 PID: 20 Comm: kworker/u2:1 Not tainted 6.6.0-rc5-dirty #56
> Hardware name: ucbbar,riscvemu-bare (DT)
> Workqueue: events_unbound io_ring_exit_work
> epc : io_pages_free+0x2a/0x58
> ra : io_rings_free+0x3a/0x50
> epc : ffffffff808811a2 ra : ffffffff80881406 sp : ffff8f80000c3cd0
> status: 0000000200000121 badaddr: 0000000000000000 cause: 000000000000000d
> [<ffffffff808811a2>] io_pages_free+0x2a/0x58
> [<ffffffff80881406>] io_rings_free+0x3a/0x50
> [<ffffffff80882176>] io_ring_exit_work+0x37e/0x424
> [<ffffffff80027234>] process_one_work+0x10c/0x1f4
> [<ffffffff8002756e>] worker_thread+0x252/0x31c
> [<ffffffff8002f5e4>] kthread+0xc4/0xe0
> [<ffffffff8000332a>] ret_from_fork+0xa/0x1c
>
> Check for a NULL array in io_pages_free(), but also clear the page counts
> when we free them to be on the safer side.
>
> Reported-by: rtm@xxxxxxxxxxxxx
> Fixes: 03d89a2de25b ("io_uring: support for user allocated memory for rings/sqes")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Jens Axboe <axboe@xxxxxxxxx>
>
> ---
>
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index d839a80a6751..8d1bc6cdfe71 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -2674,7 +2674,11 @@ static void io_pages_free(struct page ***pages, int npages)
>
> if (!pages)
> return;
> +
> page_array = *pages;
> + if (!page_array)
> + return;
> +
> for (i = 0; i < npages; i++)
> unpin_user_page(page_array[i]);
> kvfree(page_array);
> @@ -2758,7 +2762,9 @@ static void io_rings_free(struct io_ring_ctx *ctx)
> ctx->sq_sqes = NULL;
> } else {
> io_pages_free(&ctx->ring_pages, ctx->n_ring_pages);
> + ctx->n_ring_pages = 0;
> io_pages_free(&ctx->sqe_pages, ctx->n_sqe_pages);
> + ctx->n_sqe_pages = 0;
> }
> }
Reviewed-by: Jeff Moyer <jmoyer@xxxxxxxxxx>
