[PATCHSET,RFC,0/3] Add support for ring resizing

Message

Jens Axboe Oct. 22, 2024, 2:08 a.m. UTC

Hi,

Something that's come up several times over the years is how to deal
with ring sizing. The SQ ring sizing is usually trivial - it just
controls the batch submit size, and usually it's not that difficult
to just submit if the app fails to get a free SQE.

For the CQ ring, it's a different story. For networked workloads, it
can be hard to appropriately size the CQ ring without knowing exactly
how busy a given ring will be. This leads to applications grossly
over-sizing the ring, just in case, which is wasteful.

Here's a stab at supporting ring resizing. It supports resizing of
both rings, SQ and CQ, as it's really no different than just doing
the CQ ring itself. liburing has a 'resize-rings' branch with a bit
of support code, and a test case:

https://git.kernel.dk/cgit/liburing/log/?h=resize-rings

and these patches can also be found here:

https://git.kernel.dk/cgit/linux/log/?h=io_uring-ring-resize

 include/uapi/linux/io_uring.h |   3 +
 io_uring/io_uring.c           |  84 ++++++++++--------
 io_uring/io_uring.h           |   6 ++
 io_uring/register.c           | 161 ++++++++++++++++++++++++++++++++++
 4 files changed, 216 insertions(+), 38 deletions(-)

Comments

Jann Horn Oct. 24, 2024, 3:47 p.m. UTC | #1

On Tue, Oct 22, 2024 at 4:08 AM Jens Axboe <[e-mail]> wrote:
> Here's a stab at supporting ring resizing. It supports resizing of
> both rings, SQ and CQ, as it's really no different than just doing
> the CQ ring itself. liburing has a 'resize-rings' branch with a bit
> of support code, and a test case:
>
> https://git.kernel.dk/cgit/liburing/log/?h=resize-rings
>
> and these patches can also be found here:
>
> https://git.kernel.dk/cgit/linux/log/?h=io_uring-ring-resize

You'd need to properly synchronize that path with io_uring_mmap(),
right? Take a lock that prevents concurrent mmap() from accessing
ctx->ring_pages while the resize is concurrently freeing that array,
so that you don't get UAF?

And I guess ideally you'd also zap the already-mapped pages from
corresponding VMAs with something like unmap_mapping_range(), though
that won't make a difference security-wise since the pages are
refcounted by the userspace mapping anyway.

Jens Axboe Oct. 24, 2024, 4:05 p.m. UTC | #2

On 10/24/24 9:47 AM, Jann Horn wrote:
> On Tue, Oct 22, 2024 at 4:08 AM Jens Axboe <[e-mail]> wrote:
>> Here's a stab at supporting ring resizing. It supports resizing of
>> both rings, SQ and CQ, as it's really no different than just doing
>> the CQ ring itself. liburing has a 'resize-rings' branch with a bit
>> of support code, and a test case:
>>
>> https://git.kernel.dk/cgit/liburing/log/?h=resize-rings
>>
>> and these patches can also be found here:
>>
>> https://git.kernel.dk/cgit/linux/log/?h=io_uring-ring-resize
> 
> You'd need to properly synchronize that path with io_uring_mmap(),
> right? Take a lock that prevents concurrent mmap() from accessing
> ctx->ring_pages while the resize is concurrently freeing that array,
> so that you don't get UAF?

Yep indeed! It's missing the mmap_lock, I'll add that.

> And I guess ideally you'd also zap the already-mapped pages from
> corresponding VMAs with something like unmap_mapping_range(), though
> that won't make a difference security-wise since the pages are
> refcounted by the userspace mapping anyway.

Yes don't think we need to do anything there, just have userspace
unmap the old range upon return.