Re: [PATCH] io_uring: fix crash with IORING_SETUP_NO_MMAP and invalid SQ ring address

[Jeff Moyer <jmoyer@redhat.com>]

Jens Axboe <axboe@kernel.dk> 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@csail.mit.edu
> Fixes: 03d89a2de25b ("io_uring: support for user allocated memory for rings/sqes")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>
> ---
>
> 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@redhat.com>