Created attachment 134395 [details] path SUMMARY invalid memory size for io_setup syscall see sys_io_destroy for right size STEPS TO REPRODUCE 1. 2. 3. OBSERVED RESULT EXPECTED RESULT SOFTWARE/OS VERSIONS Windows: macOS: Linux/KDE Plasma: (available in About System) KDE Plasma Version: KDE Frameworks Version: Qt Version: ADDITIONAL INFORMATION
Could you provide an example that shows what was wrong before, and is fixed with this patch?
I'm sorry I cannot do example. The bug was found on big application after some time of work. It was marked as mmaped memory by input parameter. But kernel select the query size by own reasons. The real qyery size it return in nr field. In my case argument ask 1024 query size. Kernel allocates about 1900 items in the query. So after 1024 async io query we get false messages about using undefined memory. See io_destroy. It unmapped right size of query.
We have false messages like this ==13393== Invalid read of size 8 ==13393== at 0x4F8C92: io_getevents (iox_libaio.c:286) ==13393== by 0x4F94EE: iox_cbl_receive_laio (iox_libaio.c:501) .... ==13393== Address 0x7f3c37e5e008 is not stack'd, malloc'd or (recently) free'd The code example that read the events from queue. struct aio_ring *ring = (struct aio_ring *)ctx; if (ring == NULL || ring->magic != AIO_RING_MAGIC) goto do_syscall; while (i < max_nr) { unsigned head = ring->head; if (head == ring->tail) { break; } else { events[i] = ring->events[head]; ..... } A'm sorry query == queue in previous message
An actual testcase would be very welcome. I find it hard to get the actual documentation for the aio io_setup syscall. The man page does document the second argument as aio_context_t *, but from the code it looks like it actually is a aio_ring *.
Here's my take on this. The man page says DESCRIPTION Note: this page describes the raw Linux system call interface. The wrapper function provided by libaio uses a differ‐ ent type for the ctx_idp argument. See VERSIONS. The io_setup() system call creates an asynchronous I/O context suitable for concurrently processing nr_events opera‐ tions. The ctx_idp argument must not point to an AIO context that already exists, and must be initialized to 0 prior to the call. On successful creation of the AIO context, *ctx_idp is filled in with the resulting handle. We have one testcase that uses io_setup (from bug420682). This does at like the manpage above. Unfortunately (a)io_context is opaque and I can't see what is inside it in GDB. I do just see the nr_events and the pointer to the opaque io_context being passed to the syscall. From what I see in the kernel source it's just an unsigned long that will contain the context user_id. I can't see anything in the kernel related to struct aio_ring. The kernel does allocate a struct kioctx but that's in kernel memory space. Looking at pmap -x there is 00007ffff7ff4000 8 8 0 rw-s- [aio] (deleted) And that's the address that comes back in ctx. If that thing is pointing to a struct aio_ring then what I see all matches this patch. The size of both struct aio_ring and struct io_event are 32 bytes. Without the patch ARG1 is 1 so size is 64 rounded up to a page, so 4k. With the patch, r->rn is 255 so size is 32 + 32*255 or 32*256 = 8k. What that's what I saw in the pmap -x output. I can see the value of AIO_RING_MAGIC. I don't follow what is happening in aio_setup_ring nr_events += 2; /* 1 is required, 2 for good luck */ size = sizeof(struct aio_ring); size += sizeof(struct io_event) * nr_events; nr_pages = PFN_UP(size); In the TC nr_events is 1, so this will make it 3. Size will be 4*32=128. nr_pages should be 1. Then there's this nr_events = (PAGE_SIZE * nr_pages - sizeof(struct aio_ring)) / sizeof(struct io_event); I make that 127 not 255. For the mmap I see that they use the value of a loop counter after looping over nr_pages. So that should be 2. Finally ring->nr = nr_events; /* user copy */ ring->id = ~0U; I'd expect that id to be 0xffffffff. Apart from me probably not being able to read the kernel code this does look OK.