| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 16 Mar 2023 12:26:45 +0000
From: Pavel Begunkov <asml.silence@...il.com>
To: Mark Rutland <mark.rutland@....com>, Jens Axboe <axboe@...nel.dk>
Cc: io-uring@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH for-next 4/4] io_uring/rsrc: optimise registered huge
pages
On 3/16/23 12:12, Mark Rutland wrote:
> Hi,
>
> Since v6.3-rc1, when fuzzing arm64 with Syzkaller, I see a bunch of bad page
> state splats with either "nonzero pincount" or "nonzero compound_pincount", and
> bisecting led me to this patch / commit:
>
> 57bebf807e2abcf8 ("io_uring/rsrc: optimise registered huge pages")
We'll take a look, thanks for letting know
>
> The splats look like:
>
> | BUG: Bad page state in process kworker/u8:0 pfn:5c001
> | page:00000000bfda61c8 refcount:0 mapcount:0 mapping:0000000000000000 index:0x20001 pfn:0x5c001
> | head:0000000011409842 order:9 entire_mapcount:0 nr_pages_mapped:0 pincount:1
> | anon flags: 0x3fffc00000b0004(uptodate|head|mappedtodisk|swapbacked|node=0|zone=0|lastcpupid=0xffff)
> | raw: 03fffc0000000000 fffffc0000700001 ffffffff00700903 0000000100000000
> | raw: 0000000000000200 0000000000000000 00000000ffffffff 0000000000000000
> | head: 03fffc00000b0004 dead000000000100 dead000000000122 ffff00000a809dc1
> | head: 0000000000020000 0000000000000000 00000000ffffffff 0000000000000000
> | page dumped because: nonzero pincount
> | CPU: 3 PID: 9 Comm: kworker/u8:0 Not tainted 6.3.0-rc2-00001-gc6811bf0cd87 #1
> | Hardware name: linux,dummy-virt (DT)
> | Workqueue: events_unbound io_ring_exit_work
> | Call trace:
> | dump_backtrace+0x13c/0x208
> | show_stack+0x34/0x58
> | dump_stack_lvl+0x150/0x1a8
> | dump_stack+0x20/0x30
> | bad_page+0xec/0x238
> | free_tail_pages_check+0x280/0x350
> | free_pcp_prepare+0x60c/0x830
> | free_unref_page+0x50/0x498
> | free_compound_page+0xcc/0x100
> | free_transhuge_page+0x1f0/0x2b8
> | destroy_large_folio+0x80/0xc8
> | __folio_put+0xc4/0xf8
> | gup_put_folio+0xd0/0x250
> | unpin_user_page+0xcc/0x128
> | io_buffer_unmap+0xec/0x2c0
> | __io_sqe_buffers_unregister+0xa4/0x1e0
> | io_ring_exit_work+0x68c/0x1188
> | process_one_work+0x91c/0x1a58
> | worker_thread+0x48c/0xe30
> | kthread+0x278/0x2f0
> | ret_from_fork+0x10/0x20
>
> Syzkaller gave me a reliable C reproducer, which I used to bisect (note:
> DEBUG_VM needs to be selected):
>
> | // autogenerated by syzkaller (https://github.com/google/syzkaller)
> |
> | #define _GNU_SOURCE
> |
> | #include <endian.h>
> | #include <stdint.h>
> | #include <stdio.h>
> | #include <stdlib.h>
> | #include <string.h>
> | #include <sys/mman.h>
> | #include <sys/syscall.h>
> | #include <sys/types.h>
> | #include <unistd.h>
> |
> | #ifndef __NR_io_uring_register
> | #define __NR_io_uring_register 427
> | #endif
> | #ifndef __NR_io_uring_setup
> | #define __NR_io_uring_setup 425
> | #endif
> | #ifndef __NR_mmap
> | #define __NR_mmap 222
> | #endif
> |
> | #define SIZEOF_IO_URING_SQE 64
> | #define SIZEOF_IO_URING_CQE 16
> | #define SQ_HEAD_OFFSET 0
> | #define SQ_TAIL_OFFSET 64
> | #define SQ_RING_MASK_OFFSET 256
> | #define SQ_RING_ENTRIES_OFFSET 264
> | #define SQ_FLAGS_OFFSET 276
> | #define SQ_DROPPED_OFFSET 272
> | #define CQ_HEAD_OFFSET 128
> | #define CQ_TAIL_OFFSET 192
> | #define CQ_RING_MASK_OFFSET 260
> | #define CQ_RING_ENTRIES_OFFSET 268
> | #define CQ_RING_OVERFLOW_OFFSET 284
> | #define CQ_FLAGS_OFFSET 280
> | #define CQ_CQES_OFFSET 320
> |
> | struct io_sqring_offsets {
> | uint32_t head;
> | uint32_t tail;
> | uint32_t ring_mask;
> | uint32_t ring_entries;
> | uint32_t flags;
> | uint32_t dropped;
> | uint32_t array;
> | uint32_t resv1;
> | uint64_t resv2;
> | };
> |
> | struct io_cqring_offsets {
> | uint32_t head;
> | uint32_t tail;
> | uint32_t ring_mask;
> | uint32_t ring_entries;
> | uint32_t overflow;
> | uint32_t cqes;
> | uint64_t resv[2];
> | };
> |
> | struct io_uring_params {
> | uint32_t sq_entries;
> | uint32_t cq_entries;
> | uint32_t flags;
> | uint32_t sq_thread_cpu;
> | uint32_t sq_thread_idle;
> | uint32_t features;
> | uint32_t resv[4];
> | struct io_sqring_offsets sq_off;
> | struct io_cqring_offsets cq_off;
> | };
> |
> | #define IORING_OFF_SQ_RING 0
> | #define IORING_OFF_SQES 0x10000000ULL
> |
> | static long syz_io_uring_setup(volatile long a0, volatile long a1, volatile long a2, volatile long a3, volatile long a4, volatile long a5)
> | {
> | uint32_t entries = (uint32_t)a0;
> | struct io_uring_params* setup_params = (struct io_uring_params*)a1;
> | void* vma1 = (void*)a2;
> | void* vma2 = (void*)a3;
> | void** ring_ptr_out = (void**)a4;
> | void** sqes_ptr_out = (void**)a5;
> | uint32_t fd_io_uring = syscall(__NR_io_uring_setup, entries, setup_params);
> | uint32_t sq_ring_sz = setup_params->sq_off.array + setup_params->sq_entries * sizeof(uint32_t);
> | uint32_t cq_ring_sz = setup_params->cq_off.cqes + setup_params->cq_entries * SIZEOF_IO_URING_CQE;
> | uint32_t ring_sz = sq_ring_sz > cq_ring_sz ? sq_ring_sz : cq_ring_sz;
> | *ring_ptr_out = mmap(vma1, ring_sz, PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE | MAP_FIXED, fd_io_uring, IORING_OFF_SQ_RING);
> | uint32_t sqes_sz = setup_params->sq_entries * SIZEOF_IO_URING_SQE;
> | *sqes_ptr_out = mmap(vma2, sqes_sz, PROT_READ | PROT_WRITE, MAP_SHARED | MAP_POPULATE | MAP_FIXED, fd_io_uring, IORING_OFF_SQES);
> | return fd_io_uring;
> | }
> |
> | uint64_t r[1] = {0xffffffffffffffff};
> |
> | int main(void)
> | {
> | syscall(__NR_mmap, 0x1ffff000ul, 0x1000ul, 0ul, 0x32ul, -1, 0ul);
> | syscall(__NR_mmap, 0x20000000ul, 0x1000000ul, 7ul, 0x32ul, -1, 0ul);
> | syscall(__NR_mmap, 0x21000000ul, 0x1000ul, 0ul, 0x32ul, -1, 0ul);
> | intptr_t res = 0;
> | *(uint32_t*)0x20000684 = 0;
> | *(uint32_t*)0x20000688 = 0;
> | *(uint32_t*)0x2000068c = 0;
> | *(uint32_t*)0x20000690 = 0;
> | *(uint32_t*)0x20000698 = -1;
> | memset((void*)0x2000069c, 0, 12);
> | res = -1;
> | res = syz_io_uring_setup(0x2fd6, 0x20000680, 0x20ffd000, 0x20ffc000, 0x20000700, 0x20000740);
> | if (res != -1)
> | r[0] = res;
> | *(uint64_t*)0x20002840 = 0;
> | *(uint64_t*)0x20002848 = 0;
> | *(uint64_t*)0x20002850 = 0x20000840;
> | *(uint64_t*)0x20002858 = 0x1000;
> | syscall(__NR_io_uring_register, r[0], 0ul, 0x20002840ul, 2ul);
> | return 0;
> | }
>
> Thanks,
> Mark.
>
> On Wed, Feb 22, 2023 at 02:36:51PM +0000, Pavel Begunkov wrote:
>> When registering huge pages, internally io_uring will split them into
>> many PAGE_SIZE bvec entries. That's bad for performance as drivers need
>> to eventually dma-map the data and will do it individually for each bvec
>> entry. Coalesce huge pages into one large bvec.
>>
>> Signed-off-by: Pavel Begunkov <asml.silence@...il.com>
>> ---
>> io_uring/rsrc.c | 38 ++++++++++++++++++++++++++++++++------
>> 1 file changed, 32 insertions(+), 6 deletions(-)
>>
>> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
>> index ebbd2cea7582..aab1bc6883e9 100644
>> --- a/io_uring/rsrc.c
>> +++ b/io_uring/rsrc.c
>> @@ -1210,6 +1210,7 @@ static int io_sqe_buffer_register(struct io_ring_ctx *ctx, struct iovec *iov,
>> unsigned long off;
>> size_t size;
>> int ret, nr_pages, i;
>> + struct folio *folio;
>>
>> *pimu = ctx->dummy_ubuf;
>> if (!iov->iov_base)
>> @@ -1224,6 +1225,21 @@ static int io_sqe_buffer_register(struct io_ring_ctx *ctx, struct iovec *iov,
>> goto done;
>> }
>>
>> + /* If it's a huge page, try to coalesce them into a single bvec entry */
>> + if (nr_pages > 1) {
>> + folio = page_folio(pages[0]);
>> + for (i = 1; i < nr_pages; i++) {
>> + if (page_folio(pages[i]) != folio) {
>> + folio = NULL;
>> + break;
>> + }
>> + }
>> + if (folio) {
>> + folio_put_refs(folio, nr_pages - 1);
>> + nr_pages = 1;
>> + }
>> + }
>> +
>> imu = kvmalloc(struct_size(imu, bvec, nr_pages), GFP_KERNEL);
>> if (!imu)
>> goto done;
>> @@ -1236,6 +1252,17 @@ static int io_sqe_buffer_register(struct io_ring_ctx *ctx, struct iovec *iov,
>>
>> off = (unsigned long) iov->iov_base & ~PAGE_MASK;
>> size = iov->iov_len;
>> + /* store original address for later verification */
>> + imu->ubuf = (unsigned long) iov->iov_base;
>> + imu->ubuf_end = imu->ubuf + iov->iov_len;
>> + imu->nr_bvecs = nr_pages;
>> + *pimu = imu;
>> + ret = 0;
>> +
>> + if (folio) {
>> + bvec_set_page(&imu->bvec[0], pages[0], size, off);
>> + goto done;
>> + }
>> for (i = 0; i < nr_pages; i++) {
>> size_t vec_len;
>>
>> @@ -1244,12 +1271,6 @@ static int io_sqe_buffer_register(struct io_ring_ctx *ctx, struct iovec *iov,
>> off = 0;
>> size -= vec_len;
>> }
>> - /* store original address for later verification */
>> - imu->ubuf = (unsigned long) iov->iov_base;
>> - imu->ubuf_end = imu->ubuf + iov->iov_len;
>> - imu->nr_bvecs = nr_pages;
>> - *pimu = imu;
>> - ret = 0;
>> done:
>> if (ret)
>> kvfree(imu);
>> @@ -1364,6 +1385,11 @@ int io_import_fixed(int ddir, struct iov_iter *iter,
>> const struct bio_vec *bvec = imu->bvec;
>>
>> if (offset <= bvec->bv_len) {
>> + /*
>> + * Note, huge pages buffers consists of one large
>> + * bvec entry and should always go this way. The other
>> + * branch doesn't expect non PAGE_SIZE'd chunks.
>> + */
>> iter->bvec = bvec;
>> iter->nr_segs = bvec->bv_len;
>> iter->count -= offset;
>> --
>> 2.39.1
>>
>>
--
Pavel Begunkov
Powered by blists - more mailing lists