public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH] io_uring: fix crash with IORING_SETUP_NO_MMAP and invalid SQ ring address
@ 2023-10-18 14:18 Jens Axboe
  2023-10-18 15:26 ` Jeff Moyer
  0 siblings, 1 reply; 2+ messages in thread
From: Jens Axboe @ 2023-10-18 14:18 UTC (permalink / raw)
  To: io-uring; +Cc: rtm

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: [email protected]
Fixes: 03d89a2de25b ("io_uring: support for user allocated memory for rings/sqes")
Cc: [email protected]
Signed-off-by: Jens Axboe <[email protected]>

---

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;
 	}
 }
 
-- 
Jens Axboe


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] io_uring: fix crash with IORING_SETUP_NO_MMAP and invalid SQ ring address
  2023-10-18 14:18 [PATCH] io_uring: fix crash with IORING_SETUP_NO_MMAP and invalid SQ ring address Jens Axboe
@ 2023-10-18 15:26 ` Jeff Moyer
  0 siblings, 0 replies; 2+ messages in thread
From: Jeff Moyer @ 2023-10-18 15:26 UTC (permalink / raw)
  To: Jens Axboe; +Cc: io-uring, rtm

Jens Axboe <[email protected]> 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: [email protected]
> Fixes: 03d89a2de25b ("io_uring: support for user allocated memory for rings/sqes")
> Cc: [email protected]
> Signed-off-by: Jens Axboe <[email protected]>
>
> ---
>
> 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 <[email protected]>


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-10-18 15:21 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-18 14:18 [PATCH] io_uring: fix crash with IORING_SETUP_NO_MMAP and invalid SQ ring address Jens Axboe
2023-10-18 15:26 ` Jeff Moyer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox