public inbox for [email protected]
 help / color / mirror / Atom feed
* [RFC 0/1] io_uring: use valid mm in io_req_work_grab_env() in SQPOLL mode
@ 2020-06-19 23:57 Bijan Mottahedeh
  2020-06-19 23:57 ` [RFC 1/1] " Bijan Mottahedeh
  2020-06-20 22:23 ` [RFC 0/1] " Jens Axboe
  0 siblings, 2 replies; 6+ messages in thread
From: Bijan Mottahedeh @ 2020-06-19 23:57 UTC (permalink / raw)
  To: axboe; +Cc: io-uring

The liburing read-write test crashes with the strack trace below.

The problem is NULL current->mm dereference in io_req_work_grab_env().

Sending this as RFC since I'm not sure about any personality implications
of unconditionally using sqo_mm, and the proper way of failing the
request if no valid mm is found.

[  227.308192] BUG: kernel NULL pointer dereference, address: 0000000000000060
[  227.310320] #PF: supervisor write access in kernel mode
[  227.311789] #PF: error_code(0x0002) - not-present page
[  227.313170] PGD 8000000f951e7067 P4D 8000000f951e7067 PUD f9768d067 PMD 0
[  227.314918] Oops: 0002 [#1] SMP DEBUG_PAGEALLOC PTI
[  227.316094] CPU: 4 PID: 6209 Comm: io_uring-sq Not tainted 5.8.0-rc1-next-203
[  227.318050] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-4
[  227.319964] RIP: 0010:__io_queue_sqe+0x503/0x790
[  227.320706] Code: 68 00 00 00 01 49 83 be c8 00 00 00 00 75 2d 42 f6 04 bd 60
[  227.323688] RSP: 0018:ffffc90001297db8 EFLAGS: 00010202
[  227.324520] RAX: ffff888f9aa90040 RBX: ffff888f98dc8af8 RCX: 0000000000000000
[  227.325644] RDX: 0000000000000000 RSI: 0000000000001000 RDI: ffff888f98dc8b28
[  227.326767] RBP: ffffc90001297e38 R08: ffffc90001297c18 R09: 0000000000000001
[  227.327929] R10: 00000000ffffffeb R11: ffff888facb3b800 R12: 0000000000000000
[  227.329042] R13: ffff888facb3b800 R14: ffff888f98dc8a40 R15: 0000000000000001
[  227.330155] FS:  0000000000000000(0000) GS:ffff888ff0e00000(0000) knlGS:00000
[  227.331419] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  227.332338] CR2: 0000000000000060 CR3: 0000000f97bbc002 CR4: 0000000000160ee0
[  227.333449] Call Trace:
[  227.333864]  ? kvm_sched_clock_read+0xd/0x20
[  227.334537]  ? task_work_run+0x61/0x80
[  227.335151]  io_async_buf_retry+0x3b/0x50
[  227.335760]  task_work_run+0x6a/0x80
[  227.336356]  io_sq_thread+0x14e/0x320


Bijan Mottahedeh (1):
  io_uring: use valid mm in io_req_work_grab_env() in SQPOLL mode

 fs/io_uring.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

-- 
1.8.3.1


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

* [RFC 1/1] io_uring: use valid mm in io_req_work_grab_env() in SQPOLL mode
  2020-06-19 23:57 [RFC 0/1] io_uring: use valid mm in io_req_work_grab_env() in SQPOLL mode Bijan Mottahedeh
@ 2020-06-19 23:57 ` Bijan Mottahedeh
  2020-06-20  9:59   ` Pavel Begunkov
  2020-06-20 22:23 ` [RFC 0/1] " Jens Axboe
  1 sibling, 1 reply; 6+ messages in thread
From: Bijan Mottahedeh @ 2020-06-19 23:57 UTC (permalink / raw)
  To: axboe; +Cc: io-uring

If current->mm is not set in SQPOLL mode, then use ctx->sqo_mm;
otherwise fail thre request.

Signed-off-by: Bijan Mottahedeh <[email protected]>
---
 fs/io_uring.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index cb696ab..fd53ea6 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -1062,8 +1062,18 @@ static inline void io_req_work_grab_env(struct io_kiocb *req,
 					const struct io_op_def *def)
 {
 	if (!req->work.mm && def->needs_mm) {
-		mmgrab(current->mm);
-		req->work.mm = current->mm;
+		struct mm_struct *mm = current->mm;
+
+		if (!mm) {
+			if (req->ctx && req->ctx->sqo_thread)
+				mm = req->ctx->sqo_mm;
+			else
+				req->work.flags |= IO_WQ_WORK_CANCEL;
+		}
+		if (mm) {
+			mmgrab(mm);
+			req->work.mm = mm;
+		}
 	}
 	if (!req->work.creds)
 		req->work.creds = get_current_cred();
-- 
1.8.3.1


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

* Re: [RFC 1/1] io_uring: use valid mm in io_req_work_grab_env() in SQPOLL mode
  2020-06-19 23:57 ` [RFC 1/1] " Bijan Mottahedeh
@ 2020-06-20  9:59   ` Pavel Begunkov
  2020-06-20 14:22     ` Jens Axboe
  0 siblings, 1 reply; 6+ messages in thread
From: Pavel Begunkov @ 2020-06-20  9:59 UTC (permalink / raw)
  To: Bijan Mottahedeh, axboe; +Cc: io-uring

On 20/06/2020 02:57, Bijan Mottahedeh wrote:
> If current->mm is not set in SQPOLL mode, then use ctx->sqo_mm;
> otherwise fail thre request.

io_sq_thread_acquire_mm() called from io_async_buf_retry() should've
guaranteed presence of current->mm. Though, the problem could be in
"io_op_defs[req->opcode].needs_mm" check there, which is done only
for the first request in a link.

> 
> Signed-off-by: Bijan Mottahedeh <[email protected]>
> ---
>  fs/io_uring.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index cb696ab..fd53ea6 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -1062,8 +1062,18 @@ static inline void io_req_work_grab_env(struct io_kiocb *req,
>  					const struct io_op_def *def)
>  {
>  	if (!req->work.mm && def->needs_mm) {
> -		mmgrab(current->mm);
> -		req->work.mm = current->mm;
> +		struct mm_struct *mm = current->mm;
> +
> +		if (!mm) {
> +			if (req->ctx && req->ctx->sqo_thread)
> +				mm = req->ctx->sqo_mm;
> +			else
> +				req->work.flags |= IO_WQ_WORK_CANCEL;
> +		}
> +		if (mm) {
> +			mmgrab(mm);
> +			req->work.mm = mm;
> +		}
>  	}
>  	if (!req->work.creds)
>  		req->work.creds = get_current_cred();
> 

-- 
Pavel Begunkov

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

* Re: [RFC 1/1] io_uring: use valid mm in io_req_work_grab_env() in SQPOLL mode
  2020-06-20  9:59   ` Pavel Begunkov
@ 2020-06-20 14:22     ` Jens Axboe
  2020-06-22 20:48       ` Bijan Mottahedeh
  0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2020-06-20 14:22 UTC (permalink / raw)
  To: Pavel Begunkov, Bijan Mottahedeh; +Cc: io-uring

On 6/20/20 3:59 AM, Pavel Begunkov wrote:
> On 20/06/2020 02:57, Bijan Mottahedeh wrote:
>> If current->mm is not set in SQPOLL mode, then use ctx->sqo_mm;
>> otherwise fail thre request.
> 
> io_sq_thread_acquire_mm() called from io_async_buf_retry() should've
> guaranteed presence of current->mm. Though, the problem could be in
> "io_op_defs[req->opcode].needs_mm" check there, which is done only
> for the first request in a link.

Right, Bijan are you sure this isn't fixed by one of the fixes that
went upstream yesterday:

commit 9d8426a09195e2dcf2aa249de2aaadd792d491c7
Author: Jens Axboe <[email protected]>
Date:   Tue Jun 16 18:42:49 2020 -0600

    io_uring: acquire 'mm' for task_work for SQPOLL

-- 
Jens Axboe


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

* Re: [RFC 0/1] io_uring: use valid mm in io_req_work_grab_env() in SQPOLL mode
  2020-06-19 23:57 [RFC 0/1] io_uring: use valid mm in io_req_work_grab_env() in SQPOLL mode Bijan Mottahedeh
  2020-06-19 23:57 ` [RFC 1/1] " Bijan Mottahedeh
@ 2020-06-20 22:23 ` Jens Axboe
  1 sibling, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2020-06-20 22:23 UTC (permalink / raw)
  To: Bijan Mottahedeh; +Cc: io-uring

On 6/19/20 5:57 PM, Bijan Mottahedeh wrote:
> The liburing read-write test crashes with the strack trace below.
> 
> The problem is NULL current->mm dereference in io_req_work_grab_env().
> 
> Sending this as RFC since I'm not sure about any personality implications
> of unconditionally using sqo_mm, and the proper way of failing the
> request if no valid mm is found.
> 
> [  227.308192] BUG: kernel NULL pointer dereference, address: 0000000000000060
> [  227.310320] #PF: supervisor write access in kernel mode
> [  227.311789] #PF: error_code(0x0002) - not-present page
> [  227.313170] PGD 8000000f951e7067 P4D 8000000f951e7067 PUD f9768d067 PMD 0
> [  227.314918] Oops: 0002 [#1] SMP DEBUG_PAGEALLOC PTI
> [  227.316094] CPU: 4 PID: 6209 Comm: io_uring-sq Not tainted 5.8.0-rc1-next-203
> [  227.318050] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-4
> [  227.319964] RIP: 0010:__io_queue_sqe+0x503/0x790
> [  227.320706] Code: 68 00 00 00 01 49 83 be c8 00 00 00 00 75 2d 42 f6 04 bd 60
> [  227.323688] RSP: 0018:ffffc90001297db8 EFLAGS: 00010202
> [  227.324520] RAX: ffff888f9aa90040 RBX: ffff888f98dc8af8 RCX: 0000000000000000
> [  227.325644] RDX: 0000000000000000 RSI: 0000000000001000 RDI: ffff888f98dc8b28
> [  227.326767] RBP: ffffc90001297e38 R08: ffffc90001297c18 R09: 0000000000000001
> [  227.327929] R10: 00000000ffffffeb R11: ffff888facb3b800 R12: 0000000000000000
> [  227.329042] R13: ffff888facb3b800 R14: ffff888f98dc8a40 R15: 0000000000000001
> [  227.330155] FS:  0000000000000000(0000) GS:ffff888ff0e00000(0000) knlGS:00000
> [  227.331419] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  227.332338] CR2: 0000000000000060 CR3: 0000000f97bbc002 CR4: 0000000000160ee0
> [  227.333449] Call Trace:
> [  227.333864]  ? kvm_sched_clock_read+0xd/0x20
> [  227.334537]  ? task_work_run+0x61/0x80
> [  227.335151]  io_async_buf_retry+0x3b/0x50
> [  227.335760]  task_work_run+0x6a/0x80
> [  227.336356]  io_sq_thread+0x14e/0x320

Ah I missed you're running for-next, the next update should work for you,
the io_async_buf_retry() got fixed with the necessary mm grab for SQPOLL
a few days ago:

https://git.kernel.dk/cgit/linux-block/commit/?h=async-buffered.8&id=3ad1d68c04bf9555942b63b5aba31e446fdcf355

-- 
Jens Axboe


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

* Re: [RFC 1/1] io_uring: use valid mm in io_req_work_grab_env() in SQPOLL mode
  2020-06-20 14:22     ` Jens Axboe
@ 2020-06-22 20:48       ` Bijan Mottahedeh
  0 siblings, 0 replies; 6+ messages in thread
From: Bijan Mottahedeh @ 2020-06-22 20:48 UTC (permalink / raw)
  To: Jens Axboe, Pavel Begunkov; +Cc: io-uring

On 6/20/2020 7:22 AM, Jens Axboe wrote:
> On 6/20/20 3:59 AM, Pavel Begunkov wrote:
>> On 20/06/2020 02:57, Bijan Mottahedeh wrote:
>>> If current->mm is not set in SQPOLL mode, then use ctx->sqo_mm;
>>> otherwise fail thre request.
>>
>> io_sq_thread_acquire_mm() called from io_async_buf_retry() should've
>> guaranteed presence of current->mm. Though, the problem could be in
>> "io_op_defs[req->opcode].needs_mm" check there, which is done only
>> for the first request in a link.
> 
> Right, Bijan are you sure this isn't fixed by one of the fixes that
> went upstream yesterday:
> 
> commit 9d8426a09195e2dcf2aa249de2aaadd792d491c7
> Author: Jens Axboe <[email protected]>
> Date:   Tue Jun 16 18:42:49 2020 -0600
> 
>      io_uring: acquire 'mm' for task_work for SQPOLL
> 

I was running next-20200618.

Both 0618 and 0620 contain

io_uring: acquire 'mm' for task_work for SQPOLL


The commit

io_uring: support true async buffered reads, if file provides it

however is different in 0618 and 0620, the 0618 version is missing the call

if (!io_sq_thread_acquire_mm(ctx, req)) {

in io_async_buf_retry().

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/fs/io_uring.c?h=next-20200618&id=a3bb0c190b85781d7857b7a55cb9cefded5f527b
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/fs/io_uring.c?h=next-20200622&id=3ad1d68c04bf9555942b63b5aba31e446fdcf355

I pulled the 0622 and the read-write test runs fine.

--bijan

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

end of thread, other threads:[~2020-06-22 20:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-19 23:57 [RFC 0/1] io_uring: use valid mm in io_req_work_grab_env() in SQPOLL mode Bijan Mottahedeh
2020-06-19 23:57 ` [RFC 1/1] " Bijan Mottahedeh
2020-06-20  9:59   ` Pavel Begunkov
2020-06-20 14:22     ` Jens Axboe
2020-06-22 20:48       ` Bijan Mottahedeh
2020-06-20 22:23 ` [RFC 0/1] " Jens Axboe

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