* [PATCH] io_uring: don't touch 'ctx' after installing file descriptor
@ 2020-07-30 19:47 Jens Axboe
2020-07-31 7:58 ` Stefano Garzarella
0 siblings, 1 reply; 3+ messages in thread
From: Jens Axboe @ 2020-07-30 19:47 UTC (permalink / raw)
To: io-uring
As soon as we install the file descriptor, we have to assume that it
can get arbitrarily closed. We currently account memory (and note that
we did) after installing the ring fd, which means that it could be a
potential use-after-free condition if the fd is closed right after
being installed, but before we fiddle with the ctx.
In fact, syzbot reported this exact scenario:
BUG: KASAN: use-after-free in io_account_mem fs/io_uring.c:7397 [inline]
BUG: KASAN: use-after-free in io_uring_create fs/io_uring.c:8369 [inline]
BUG: KASAN: use-after-free in io_uring_setup+0x2797/0x2910 fs/io_uring.c:8400
Read of size 1 at addr ffff888087a41044 by task syz-executor.5/18145
CPU: 0 PID: 18145 Comm: syz-executor.5 Not tainted 5.8.0-rc7-next-20200729-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x18f/0x20d lib/dump_stack.c:118
print_address_description.constprop.0.cold+0xae/0x497 mm/kasan/report.c:383
__kasan_report mm/kasan/report.c:513 [inline]
kasan_report.cold+0x1f/0x37 mm/kasan/report.c:530
io_account_mem fs/io_uring.c:7397 [inline]
io_uring_create fs/io_uring.c:8369 [inline]
io_uring_setup+0x2797/0x2910 fs/io_uring.c:8400
do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x45c429
Code: 8d b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 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 0f 83 5b b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f8f121d0c78 EFLAGS: 00000246 ORIG_RAX: 00000000000001a9
RAX: ffffffffffffffda RBX: 0000000000008540 RCX: 000000000045c429
RDX: 0000000000000000 RSI: 0000000020000040 RDI: 0000000000000196
RBP: 000000000078bf38 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 000000000078bf0c
R13: 00007fff86698cff R14: 00007f8f121d19c0 R15: 000000000078bf0c
Move the accounting of the ring used locked memory before we get and
install the ring file descriptor.
Cc: stable@vger.kernel.org
Reported-by: syzbot+9d46305e76057f30c74e@syzkaller.appspotmail.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
diff --git a/fs/io_uring.c b/fs/io_uring.c
index fabf0b692384..33702f3b5af8 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -8329,6 +8329,15 @@ static int io_uring_create(unsigned entries, struct io_uring_params *p,
ret = -EFAULT;
goto err;
}
+
+ /*
+ * Account memory _before_ installing the file descriptor. Once
+ * the descriptor is installed, it can get closed at any time.
+ */
+ io_account_mem(ctx, ring_pages(p->sq_entries, p->cq_entries),
+ ACCT_LOCKED);
+ ctx->limit_mem = limit_mem;
+
/*
* Install ring fd as the very last thing, so we don't risk someone
* having closed it before we finish setup
@@ -8338,9 +8347,6 @@ static int io_uring_create(unsigned entries, struct io_uring_params *p,
goto err;
trace_io_uring_create(ret, ctx, p->sq_entries, p->cq_entries, p->flags);
- io_account_mem(ctx, ring_pages(p->sq_entries, p->cq_entries),
- ACCT_LOCKED);
- ctx->limit_mem = limit_mem;
return ret;
err:
io_ring_ctx_wait_and_kill(ctx);
--
Jens Axboe
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] io_uring: don't touch 'ctx' after installing file descriptor
2020-07-30 19:47 [PATCH] io_uring: don't touch 'ctx' after installing file descriptor Jens Axboe
@ 2020-07-31 7:58 ` Stefano Garzarella
2020-07-31 14:24 ` Jens Axboe
0 siblings, 1 reply; 3+ messages in thread
From: Stefano Garzarella @ 2020-07-31 7:58 UTC (permalink / raw)
To: Jens Axboe; +Cc: io-uring
On Thu, Jul 30, 2020 at 01:47:52PM -0600, Jens Axboe wrote:
> As soon as we install the file descriptor, we have to assume that it
> can get arbitrarily closed. We currently account memory (and note that
> we did) after installing the ring fd, which means that it could be a
> potential use-after-free condition if the fd is closed right after
> being installed, but before we fiddle with the ctx.
>
> In fact, syzbot reported this exact scenario:
>
> BUG: KASAN: use-after-free in io_account_mem fs/io_uring.c:7397 [inline]
> BUG: KASAN: use-after-free in io_uring_create fs/io_uring.c:8369 [inline]
> BUG: KASAN: use-after-free in io_uring_setup+0x2797/0x2910 fs/io_uring.c:8400
> Read of size 1 at addr ffff888087a41044 by task syz-executor.5/18145
>
> CPU: 0 PID: 18145 Comm: syz-executor.5 Not tainted 5.8.0-rc7-next-20200729-syzkaller #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x18f/0x20d lib/dump_stack.c:118
> print_address_description.constprop.0.cold+0xae/0x497 mm/kasan/report.c:383
> __kasan_report mm/kasan/report.c:513 [inline]
> kasan_report.cold+0x1f/0x37 mm/kasan/report.c:530
> io_account_mem fs/io_uring.c:7397 [inline]
> io_uring_create fs/io_uring.c:8369 [inline]
> io_uring_setup+0x2797/0x2910 fs/io_uring.c:8400
> do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
> entry_SYSCALL_64_after_hwframe+0x44/0xa9
> RIP: 0033:0x45c429
> Code: 8d b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 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 0f 83 5b b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007f8f121d0c78 EFLAGS: 00000246 ORIG_RAX: 00000000000001a9
> RAX: ffffffffffffffda RBX: 0000000000008540 RCX: 000000000045c429
> RDX: 0000000000000000 RSI: 0000000020000040 RDI: 0000000000000196
> RBP: 000000000078bf38 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 000000000078bf0c
> R13: 00007fff86698cff R14: 00007f8f121d19c0 R15: 000000000078bf0c
>
> Move the accounting of the ring used locked memory before we get and
> install the ring file descriptor.
Maybe we can add:
Fixes: 309758254ea6 ("io_uring: report pinned memory usage")
The patch LGTM:
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Thanks,
Stefano
>
> Cc: stable@vger.kernel.org
> Reported-by: syzbot+9d46305e76057f30c74e@syzkaller.appspotmail.com
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>
> ---
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index fabf0b692384..33702f3b5af8 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -8329,6 +8329,15 @@ static int io_uring_create(unsigned entries, struct io_uring_params *p,
> ret = -EFAULT;
> goto err;
> }
> +
> + /*
> + * Account memory _before_ installing the file descriptor. Once
> + * the descriptor is installed, it can get closed at any time.
> + */
> + io_account_mem(ctx, ring_pages(p->sq_entries, p->cq_entries),
> + ACCT_LOCKED);
> + ctx->limit_mem = limit_mem;
> +
> /*
> * Install ring fd as the very last thing, so we don't risk someone
> * having closed it before we finish setup
> @@ -8338,9 +8347,6 @@ static int io_uring_create(unsigned entries, struct io_uring_params *p,
> goto err;
>
> trace_io_uring_create(ret, ctx, p->sq_entries, p->cq_entries, p->flags);
> - io_account_mem(ctx, ring_pages(p->sq_entries, p->cq_entries),
> - ACCT_LOCKED);
> - ctx->limit_mem = limit_mem;
> return ret;
> err:
> io_ring_ctx_wait_and_kill(ctx);
>
> --
> Jens Axboe
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] io_uring: don't touch 'ctx' after installing file descriptor
2020-07-31 7:58 ` Stefano Garzarella
@ 2020-07-31 14:24 ` Jens Axboe
0 siblings, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2020-07-31 14:24 UTC (permalink / raw)
To: Stefano Garzarella; +Cc: io-uring
On 7/31/20 1:58 AM, Stefano Garzarella wrote:
> On Thu, Jul 30, 2020 at 01:47:52PM -0600, Jens Axboe wrote:
>> As soon as we install the file descriptor, we have to assume that it
>> can get arbitrarily closed. We currently account memory (and note that
>> we did) after installing the ring fd, which means that it could be a
>> potential use-after-free condition if the fd is closed right after
>> being installed, but before we fiddle with the ctx.
>>
>> In fact, syzbot reported this exact scenario:
>>
>> BUG: KASAN: use-after-free in io_account_mem fs/io_uring.c:7397 [inline]
>> BUG: KASAN: use-after-free in io_uring_create fs/io_uring.c:8369 [inline]
>> BUG: KASAN: use-after-free in io_uring_setup+0x2797/0x2910 fs/io_uring.c:8400
>> Read of size 1 at addr ffff888087a41044 by task syz-executor.5/18145
>>
>> CPU: 0 PID: 18145 Comm: syz-executor.5 Not tainted 5.8.0-rc7-next-20200729-syzkaller #0
>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
>> Call Trace:
>> __dump_stack lib/dump_stack.c:77 [inline]
>> dump_stack+0x18f/0x20d lib/dump_stack.c:118
>> print_address_description.constprop.0.cold+0xae/0x497 mm/kasan/report.c:383
>> __kasan_report mm/kasan/report.c:513 [inline]
>> kasan_report.cold+0x1f/0x37 mm/kasan/report.c:530
>> io_account_mem fs/io_uring.c:7397 [inline]
>> io_uring_create fs/io_uring.c:8369 [inline]
>> io_uring_setup+0x2797/0x2910 fs/io_uring.c:8400
>> do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
>> entry_SYSCALL_64_after_hwframe+0x44/0xa9
>> RIP: 0033:0x45c429
>> Code: 8d b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 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 0f 83 5b b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
>> RSP: 002b:00007f8f121d0c78 EFLAGS: 00000246 ORIG_RAX: 00000000000001a9
>> RAX: ffffffffffffffda RBX: 0000000000008540 RCX: 000000000045c429
>> RDX: 0000000000000000 RSI: 0000000020000040 RDI: 0000000000000196
>> RBP: 000000000078bf38 R08: 0000000000000000 R09: 0000000000000000
>> R10: 0000000000000000 R11: 0000000000000246 R12: 000000000078bf0c
>> R13: 00007fff86698cff R14: 00007f8f121d19c0 R15: 000000000078bf0c
>>
>> Move the accounting of the ring used locked memory before we get and
>> install the ring file descriptor.
>
> Maybe we can add:
> Fixes: 309758254ea6 ("io_uring: report pinned memory usage")
For sure, I just checked and had sort of assumed this existed in earlier
versions too, but it is indeed a recent issue. I'll add the Fixes tag
and your Reviewed-by, thanks!
--
Jens Axboe
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-07-31 14:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-30 19:47 [PATCH] io_uring: don't touch 'ctx' after installing file descriptor Jens Axboe
2020-07-31 7:58 ` Stefano Garzarella
2020-07-31 14:24 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).