public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH] io_uring: REQ_F_FORCE_ASYNC prep done too late
@ 2020-03-19  3:51 Jens Axboe
  2020-03-19 10:41 ` Pavel Begunkov
  0 siblings, 1 reply; 3+ messages in thread
From: Jens Axboe @ 2020-03-19  3:51 UTC (permalink / raw)
  To: io-uring; +Cc: Pavel Begunkov

A previous patch ensured that we always prepped requests that are
forced async, but it did so too late in the process. This can result
in 'sqe' already being NULL by the time we get to it:

BUG: kernel NULL pointer dereference, address: 0000000000000008
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 0 P4D 0 
Oops: 0000 [#1] PREEMPT SMP
CPU: 2 PID: 331 Comm: read-write Not tainted 5.6.0-rc5+ #5846
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
RIP: 0010:io_prep_rw+0x37/0x250
Code: 41 89 d4 55 48 89 f5 53 48 8b 0f 48 89 fb 4c 8b 6f 50 48 8b 41 20 0f b7 00 66 25 00 f0 66 3d 00 80 75 07 81 4f 68 00 40 00 00 <48> 8b 45 08 48 83 f8 ff 48 89 43 08 0f 84 c6 01 00 00 8b 41 34 85
RSP: 0018:ffffc900003ebce8 EFLAGS: 00010206
RAX: 0000000000008000 RBX: ffff8881adf87b00 RCX: ffff8881b7db4f00
RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffff8881adf87b00
RBP: 0000000000000000 R08: ffff8881adf87b88 R09: ffff8881adf87b88
R10: 0000000000001000 R11: ffffea0006ddee08 R12: 0000000000000001
R13: ffff8881b325c000 R14: 0000000000000000 R15: ffff8881b325c000
FS:  00007f642a59e540(0000) GS:ffff8881b9d00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000008 CR3: 00000001b3674004 CR4: 00000000001606e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 io_write_prep+0x1c/0x100
 io_queue_sqe+0x8f/0x2a0
 io_submit_sqes+0x450/0xa10
 __do_sys_io_uring_enter+0x272/0x610
 ? ksys_mmap_pgoff+0x15d/0x1f0
 do_syscall_64+0x42/0x100
 entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x7f642a4d0f8d
Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d d3 4e 0c 00 f7 d8 64 89 01 48

Fix this by ensuring we do prep at io_submit_sqe() time, where we
know we still have the original sqe.

Fixes: 1118591ab883 ("io_uring: prep req when do IOSQE_ASYNC")
Signed-off-by: Jens Axboe <[email protected]>

---

diff --git a/fs/io_uring.c b/fs/io_uring.c
index e6049546e77c..ff35f5ac91ea 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5528,15 +5528,11 @@ static void io_queue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	ret = io_req_defer(req, sqe);
 	if (ret) {
 		if (ret != -EIOCBQUEUED) {
-fail_req:
 			io_cqring_add_event(req, ret);
 			req_set_fail_links(req);
 			io_double_put_req(req);
 		}
 	} else if (req->flags & REQ_F_FORCE_ASYNC) {
-		ret = io_req_defer_prep(req, sqe);
-		if (unlikely(ret < 0))
-			goto fail_req;
 		/*
 		 * Never try inline submit of IOSQE_ASYNC is set, go straight
 		 * to async execution.
@@ -5650,12 +5646,21 @@ static bool io_submit_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe,
 			req->flags |= REQ_F_IO_DRAIN;
 			req->ctx->drain_next = 0;
 		}
+		if (sqe_flags & REQ_F_FORCE_ASYNC) {
+			ret = io_req_defer_prep(req, sqe);
+			if (ret < 0)
+				goto err_req;
+			/* prep done */
+			sqe = NULL;
+		}
 		if (sqe_flags & (IOSQE_IO_LINK|IOSQE_IO_HARDLINK)) {
 			req->flags |= REQ_F_LINK;
 			INIT_LIST_HEAD(&req->link_list);
-			ret = io_req_defer_prep(req, sqe);
-			if (ret)
-				req->flags |= REQ_F_FAIL_LINK;
+			if (sqe) {
+				ret = io_req_defer_prep(req, sqe);
+				if (ret)
+					req->flags |= REQ_F_FAIL_LINK;
+			}
 			*link = req;
 		} else {
 			io_queue_sqe(req, sqe);

-- 
Jens Axboe


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

* Re: [PATCH] io_uring: REQ_F_FORCE_ASYNC prep done too late
  2020-03-19  3:51 [PATCH] io_uring: REQ_F_FORCE_ASYNC prep done too late Jens Axboe
@ 2020-03-19 10:41 ` Pavel Begunkov
  2020-03-19 14:48   ` Jens Axboe
  0 siblings, 1 reply; 3+ messages in thread
From: Pavel Begunkov @ 2020-03-19 10:41 UTC (permalink / raw)
  To: Jens Axboe, io-uring

On 3/19/2020 6:51 AM, Jens Axboe wrote:
> A previous patch ensured that we always prepped requests that are
> forced async, but it did so too late in the process. This can result
> in 'sqe' already being NULL by the time we get to it:

Isn't it fixed by f1d96a8fcbbbb ("io_uring: NULL-deref for
IOSQE_{ASYNC,DRAIN}")? BTW, the same can happen with draining in
io_req_defer() -> io_req_defer_prep().

Can't look through your patches/RFC properly, but I will try do that
this weekends.


> BUG: kernel NULL pointer dereference, address: 0000000000000008
> #PF: supervisor read access in kernel mode
> #PF: error_code(0x0000) - not-present page
> PGD 0 P4D 0 
> Oops: 0000 [#1] PREEMPT SMP
> CPU: 2 PID: 331 Comm: read-write Not tainted 5.6.0-rc5+ #5846
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
> RIP: 0010:io_prep_rw+0x37/0x250
> Code: 41 89 d4 55 48 89 f5 53 48 8b 0f 48 89 fb 4c 8b 6f 50 48 8b 41 20 0f b7 00 66 25 00 f0 66 3d 00 80 75 07 81 4f 68 00 40 00 00 <48> 8b 45 08 48 83 f8 ff 48 89 43 08 0f 84 c6 01 00 00 8b 41 34 85
> RSP: 0018:ffffc900003ebce8 EFLAGS: 00010206
> RAX: 0000000000008000 RBX: ffff8881adf87b00 RCX: ffff8881b7db4f00
> RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffff8881adf87b00
> RBP: 0000000000000000 R08: ffff8881adf87b88 R09: ffff8881adf87b88
> R10: 0000000000001000 R11: ffffea0006ddee08 R12: 0000000000000001
> R13: ffff8881b325c000 R14: 0000000000000000 R15: ffff8881b325c000
> FS:  00007f642a59e540(0000) GS:ffff8881b9d00000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000008 CR3: 00000001b3674004 CR4: 00000000001606e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
>  io_write_prep+0x1c/0x100
>  io_queue_sqe+0x8f/0x2a0
>  io_submit_sqes+0x450/0xa10
>  __do_sys_io_uring_enter+0x272/0x610
>  ? ksys_mmap_pgoff+0x15d/0x1f0
>  do_syscall_64+0x42/0x100
>  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> RIP: 0033:0x7f642a4d0f8d
> Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d d3 4e 0c 00 f7 d8 64 89 01 48
> 
> Fix this by ensuring we do prep at io_submit_sqe() time, where we
> know we still have the original sqe.
> 
> Fixes: 1118591ab883 ("io_uring: prep req when do IOSQE_ASYNC")
> Signed-off-by: Jens Axboe <[email protected]>
> 
> ---
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index e6049546e77c..ff35f5ac91ea 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -5528,15 +5528,11 @@ static void io_queue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>  	ret = io_req_defer(req, sqe);
>  	if (ret) {
>  		if (ret != -EIOCBQUEUED) {
> -fail_req:
>  			io_cqring_add_event(req, ret);
>  			req_set_fail_links(req);
>  			io_double_put_req(req);
>  		}
>  	} else if (req->flags & REQ_F_FORCE_ASYNC) {
> -		ret = io_req_defer_prep(req, sqe);
> -		if (unlikely(ret < 0))
> -			goto fail_req;
>  		/*
>  		 * Never try inline submit of IOSQE_ASYNC is set, go straight
>  		 * to async execution.
> @@ -5650,12 +5646,21 @@ static bool io_submit_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe,
>  			req->flags |= REQ_F_IO_DRAIN;
>  			req->ctx->drain_next = 0;
>  		}
> +		if (sqe_flags & REQ_F_FORCE_ASYNC) {
> +			ret = io_req_defer_prep(req, sqe);
> +			if (ret < 0)
> +				goto err_req;
> +			/* prep done */
> +			sqe = NULL;
> +		}
>  		if (sqe_flags & (IOSQE_IO_LINK|IOSQE_IO_HARDLINK)) {
>  			req->flags |= REQ_F_LINK;
>  			INIT_LIST_HEAD(&req->link_list);
> -			ret = io_req_defer_prep(req, sqe);
> -			if (ret)
> -				req->flags |= REQ_F_FAIL_LINK;
> +			if (sqe) {
> +				ret = io_req_defer_prep(req, sqe);
> +				if (ret)
> +					req->flags |= REQ_F_FAIL_LINK;
> +			}
>  			*link = req;
>  		} else {
>  			io_queue_sqe(req, sqe);
> 

-- 
Pavel Begunkov

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

* Re: [PATCH] io_uring: REQ_F_FORCE_ASYNC prep done too late
  2020-03-19 10:41 ` Pavel Begunkov
@ 2020-03-19 14:48   ` Jens Axboe
  0 siblings, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2020-03-19 14:48 UTC (permalink / raw)
  To: Pavel Begunkov, io-uring

On 3/19/20 4:41 AM, Pavel Begunkov wrote:
> On 3/19/2020 6:51 AM, Jens Axboe wrote:
>> A previous patch ensured that we always prepped requests that are
>> forced async, but it did so too late in the process. This can result
>> in 'sqe' already being NULL by the time we get to it:
> 
> Isn't it fixed by f1d96a8fcbbbb ("io_uring: NULL-deref for
> IOSQE_{ASYNC,DRAIN}")? BTW, the same can happen with draining in
> io_req_defer() -> io_req_defer_prep().
> 
> Can't look through your patches/RFC properly, but I will try do that
> this weekends.

Ah I think that it is, I was running the 5.7 branch and that one
doesn't have that patch. Disregard this one, I think we're fine.

-- 
Jens Axboe


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

end of thread, other threads:[~2020-03-19 14:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-19  3:51 [PATCH] io_uring: REQ_F_FORCE_ASYNC prep done too late Jens Axboe
2020-03-19 10:41 ` Pavel Begunkov
2020-03-19 14:48   ` Jens Axboe

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