* [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