[PATCH 1/1] io_uring: more graceful request alloc OOM

[PATCH 1/1] io_uring: more graceful request alloc OOM

[Pavel Begunkov]

It's ok for io_uring request allocation to fail, however there are
reports that it starts killing tasks instead of just returning back
to the userspace. Add __GFP_NORETRY, so it doesn't trigger OOM killer.

Cc: stable@vger.kernel.org
Fixes: 2b188cc1bb857 ("Add io_uring IO interface")
Reported-by: yang lan <lanyang0908@gmail.com>
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/io_uring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index dab09f568294..ad34a4320dab 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1073,7 +1073,7 @@ static void io_flush_cached_locked_reqs(struct io_ring_ctx *ctx,
 __cold bool __io_alloc_req_refill(struct io_ring_ctx *ctx)
 	__must_hold(&ctx->uring_lock)
 {
-	gfp_t gfp = GFP_KERNEL | __GFP_NOWARN;
+	gfp_t gfp = GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY;
 	void *reqs[IO_REQ_ALLOC_BATCH];
 	int ret, i;
 
-- 
2.40.0

Re: [PATCH 1/1] io_uring: more graceful request alloc OOM

[Jens Axboe]

On Fri, 19 May 2023 15:05:14 +0100, Pavel Begunkov wrote:
> It's ok for io_uring request allocation to fail, however there are
> reports that it starts killing tasks instead of just returning back
> to the userspace. Add __GFP_NORETRY, so it doesn't trigger OOM killer.
> 
> 

Applied, thanks!

[1/1] io_uring: more graceful request alloc OOM
      (no commit info)

Best regards,
-- 
Jens Axboe

Re: [PATCH 1/1] io_uring: more graceful request alloc OOM

[yang lan]

Hi,

Thanks for your response.

But I applied this patch to LTS kernel 5.10.180, it can still trigger this bug.

--- io_uring/io_uring.c.back    2023-05-20 17:11:25.870550438 +0800
+++ io_uring/io_uring.c 2023-05-20 16:35:24.265846283 +0800
@@ -1970,7 +1970,7 @@
static struct io_kiocb *io_alloc_req(struct io_ring_ctx *ctx)
        __must_hold(&ctx->uring_lock)
 {
        struct io_submit_state *state = &ctx->submit_state;
-       gfp_t gfp = GFP_KERNEL | __GFP_NOWARN;
+       gfp_t gfp = GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY;
        int ret, i;

        BUILD_BUG_ON(ARRAY_SIZE(state->reqs) < IO_REQ_ALLOC_BATCH);

The io_uring.c.back is the original file.
Do I apply this patch wrong?

Regards,

Yang

Pavel Begunkov <asml.silence@gmail.com> 于2023年5月19日周五 22:06写道：
>
> It's ok for io_uring request allocation to fail, however there are
> reports that it starts killing tasks instead of just returning back
> to the userspace. Add __GFP_NORETRY, so it doesn't trigger OOM killer.
>
> Cc: stable@vger.kernel.org
> Fixes: 2b188cc1bb857 ("Add io_uring IO interface")
> Reported-by: yang lan <lanyang0908@gmail.com>
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  io_uring/io_uring.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index dab09f568294..ad34a4320dab 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -1073,7 +1073,7 @@ static void io_flush_cached_locked_reqs(struct io_ring_ctx *ctx,
>  __cold bool __io_alloc_req_refill(struct io_ring_ctx *ctx)
>         __must_hold(&ctx->uring_lock)
>  {
> -       gfp_t gfp = GFP_KERNEL | __GFP_NOWARN;
> +       gfp_t gfp = GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY;
>         void *reqs[IO_REQ_ALLOC_BATCH];
>         int ret, i;
>
> --
> 2.40.0
>

Re: [PATCH 1/1] io_uring: more graceful request alloc OOM

[Pavel Begunkov]

On 5/20/23 10:38, yang lan wrote:
> Hi,
> 
> Thanks for your response.
> 
> But I applied this patch to LTS kernel 5.10.180, it can still trigger this bug.
> 
> --- io_uring/io_uring.c.back    2023-05-20 17:11:25.870550438 +0800
> +++ io_uring/io_uring.c 2023-05-20 16:35:24.265846283 +0800
> @@ -1970,7 +1970,7 @@
> static struct io_kiocb *io_alloc_req(struct io_ring_ctx *ctx)
>          __must_hold(&ctx->uring_lock)
>   {
>          struct io_submit_state *state = &ctx->submit_state;
> -       gfp_t gfp = GFP_KERNEL | __GFP_NOWARN;
> +       gfp_t gfp = GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY;
>          int ret, i;
> 
>          BUILD_BUG_ON(ARRAY_SIZE(state->reqs) < IO_REQ_ALLOC_BATCH);
> 
> The io_uring.c.back is the original file.
> Do I apply this patch wrong?

The patch looks fine. I run a self-written test before
sending with 6.4, worked as expected. I need to run the syz
test, maybe it shifted to another spot, e.g. in provided
buffers.

-- 
Pavel Begunkov

Re: [PATCH 1/1] io_uring: more graceful request alloc OOM

[yang lan]

Hi,

Thanks. I'm also analyzing the root cause of this bug.

By the way, can I apply for a CVE? And should I submit a request to
some official organizations, such as Openwall, etc?

Regards,

Yang

Pavel Begunkov <asml.silence@gmail.com> 于2023年5月22日周一 08:45写道：
>
> On 5/20/23 10:38, yang lan wrote:
> > Hi,
> >
> > Thanks for your response.
> >
> > But I applied this patch to LTS kernel 5.10.180, it can still trigger this bug.
> >
> > --- io_uring/io_uring.c.back    2023-05-20 17:11:25.870550438 +0800
> > +++ io_uring/io_uring.c 2023-05-20 16:35:24.265846283 +0800
> > @@ -1970,7 +1970,7 @@
> > static struct io_kiocb *io_alloc_req(struct io_ring_ctx *ctx)
> >          __must_hold(&ctx->uring_lock)
> >   {
> >          struct io_submit_state *state = &ctx->submit_state;
> > -       gfp_t gfp = GFP_KERNEL | __GFP_NOWARN;
> > +       gfp_t gfp = GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY;
> >          int ret, i;
> >
> >          BUILD_BUG_ON(ARRAY_SIZE(state->reqs) < IO_REQ_ALLOC_BATCH);
> >
> > The io_uring.c.back is the original file.
> > Do I apply this patch wrong?
>
> The patch looks fine. I run a self-written test before
> sending with 6.4, worked as expected. I need to run the syz
> test, maybe it shifted to another spot, e.g. in provided
> buffers.
>
> --
> Pavel Begunkov

Re: [PATCH 1/1] io_uring: more graceful request alloc OOM

[Pavel Begunkov]

On 5/22/23 08:55, yang lan wrote:
> Hi,
> 
> Thanks. I'm also analyzing the root cause of this bug.

The repro indeed triggers, this time in another place. Though
when I patch all of them it would fail somewhere else, like in
ext4 on a pagefault.

We can add a couple more those "don't oom but fail" and some
niceness around, but I think it's fine as it is as allocations
are under cgroup. If admin cares about collision b/w users there
should be cgroups, so allocations of one don't affect another. And
if a user pushes it to the limit and oom's itself and gets OOM,
that should be fine.

> By the way, can I apply for a CVE? And should I submit a request to
> some official organizations, such as Openwall, etc?

Sorry, but we cannot help you here. We don't deal with CVEs.

That aside, I'm not even sure it's CVE'able because it shouldn't
be exploitable in a configured environment (unless it is). But
I'm not an expert in that so might be wrong.



> Pavel Begunkov <asml.silence@gmail.com> 于2023年5月22日周一 08:45写道：
>>
>> On 5/20/23 10:38, yang lan wrote:
>>> Hi,
>>>
>>> Thanks for your response.
>>>
>>> But I applied this patch to LTS kernel 5.10.180, it can still trigger this bug.
>>>
>>> --- io_uring/io_uring.c.back    2023-05-20 17:11:25.870550438 +0800
>>> +++ io_uring/io_uring.c 2023-05-20 16:35:24.265846283 +0800
>>> @@ -1970,7 +1970,7 @@
>>> static struct io_kiocb *io_alloc_req(struct io_ring_ctx *ctx)
>>>           __must_hold(&ctx->uring_lock)
>>>    {
>>>           struct io_submit_state *state = &ctx->submit_state;
>>> -       gfp_t gfp = GFP_KERNEL | __GFP_NOWARN;
>>> +       gfp_t gfp = GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY;
>>>           int ret, i;
>>>
>>>           BUILD_BUG_ON(ARRAY_SIZE(state->reqs) < IO_REQ_ALLOC_BATCH);
>>>
>>> The io_uring.c.back is the original file.
>>> Do I apply this patch wrong?
>>
>> The patch looks fine. I run a self-written test before
>> sending with 6.4, worked as expected. I need to run the syz
>> test, maybe it shifted to another spot, e.g. in provided
>> buffers.
>>
>> --
>> Pavel Begunkov

-- 
Pavel Begunkov

Re: [PATCH 1/1] io_uring: more graceful request alloc OOM

[yang lan]

Hi,

Thanks.

Regards,

Yang

Pavel Begunkov <asml.silence@gmail.com> 于2023年5月23日周二 20:15写道：
>
> On 5/22/23 08:55, yang lan wrote:
> > Hi,
> >
> > Thanks. I'm also analyzing the root cause of this bug.
>
> The repro indeed triggers, this time in another place. Though
> when I patch all of them it would fail somewhere else, like in
> ext4 on a pagefault.
>
> We can add a couple more those "don't oom but fail" and some
> niceness around, but I think it's fine as it is as allocations
> are under cgroup. If admin cares about collision b/w users there
> should be cgroups, so allocations of one don't affect another. And
> if a user pushes it to the limit and oom's itself and gets OOM,
> that should be fine.
>
> > By the way, can I apply for a CVE? And should I submit a request to
> > some official organizations, such as Openwall, etc?
>
> Sorry, but we cannot help you here. We don't deal with CVEs.
>
> That aside, I'm not even sure it's CVE'able because it shouldn't
> be exploitable in a configured environment (unless it is). But
> I'm not an expert in that so might be wrong.
>
>
>
> > Pavel Begunkov <asml.silence@gmail.com> 于2023年5月22日周一 08:45写道：
> >>
> >> On 5/20/23 10:38, yang lan wrote:
> >>> Hi,
> >>>
> >>> Thanks for your response.
> >>>
> >>> But I applied this patch to LTS kernel 5.10.180, it can still trigger this bug.
> >>>
> >>> --- io_uring/io_uring.c.back    2023-05-20 17:11:25.870550438 +0800
> >>> +++ io_uring/io_uring.c 2023-05-20 16:35:24.265846283 +0800
> >>> @@ -1970,7 +1970,7 @@
> >>> static struct io_kiocb *io_alloc_req(struct io_ring_ctx *ctx)
> >>>           __must_hold(&ctx->uring_lock)
> >>>    {
> >>>           struct io_submit_state *state = &ctx->submit_state;
> >>> -       gfp_t gfp = GFP_KERNEL | __GFP_NOWARN;
> >>> +       gfp_t gfp = GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY;
> >>>           int ret, i;
> >>>
> >>>           BUILD_BUG_ON(ARRAY_SIZE(state->reqs) < IO_REQ_ALLOC_BATCH);
> >>>
> >>> The io_uring.c.back is the original file.
> >>> Do I apply this patch wrong?
> >>
> >> The patch looks fine. I run a self-written test before
> >> sending with 6.4, worked as expected. I need to run the syz
> >> test, maybe it shifted to another spot, e.g. in provided
> >> buffers.
> >>
> >> --
> >> Pavel Begunkov
>
> --
> Pavel Begunkov