On Wed, Jan 30, 2019 at 12:06 AM Jens Axboe <axboe@xxxxxxxxx> wrote: > On 1/29/19 4:03 PM, Jann Horn wrote: > > On Tue, Jan 29, 2019 at 11:56 PM Jens Axboe <axboe@xxxxxxxxx> wrote: > >> On 1/29/19 3:44 PM, Jann Horn wrote: > >>> On Tue, Jan 29, 2019 at 8:27 PM Jens Axboe <axboe@xxxxxxxxx> wrote: > >>>> If we have fixed user buffers, we can map them into the kernel when we > >>>> setup the io_context. That avoids the need to do get_user_pages() for > >>>> each and every IO. > >>>> > >>>> To utilize this feature, the application must call io_uring_register() > >>>> after having setup an io_uring context, passing in > >>>> IORING_REGISTER_BUFFERS as the opcode. The argument must be a pointer > >>>> to an iovec array, and the nr_args should contain how many iovecs the > >>>> application wishes to map. > >>>> > >>>> If successful, these buffers are now mapped into the kernel, eligible > >>>> for IO. To use these fixed buffers, the application must use the > >>>> IORING_OP_READ_FIXED and IORING_OP_WRITE_FIXED opcodes, and then > >>>> set sqe->index to the desired buffer index. sqe->addr..sqe->addr+seq->len > >>>> must point to somewhere inside the indexed buffer. > >>>> > >>>> The application may register buffers throughout the lifetime of the > >>>> io_uring context. It can call io_uring_register() with > >>>> IORING_UNREGISTER_BUFFERS as the opcode to unregister the current set of > >>>> buffers, and then register a new set. The application need not > >>>> unregister buffers explicitly before shutting down the io_uring context. > > [...] > >>>> + imu = &ctx->user_bufs[index]; > >>>> + buf_addr = READ_ONCE(sqe->addr); > >>>> + if (buf_addr < imu->ubuf || buf_addr + len > imu->ubuf + imu->len) > >>> > >>> This can wrap around if `buf_addr` or `len` is very big, right? Then > >>> you e.g. get past the first check because `buf_addr` is sufficiently > >>> big, and get past the second check because `buf_addr + len` wraps > >>> around and becomes small. > >> > >> Good point. I wonder if we have a verification helper for something like > >> this? > > > > check_add_overflow() exists, I guess that might help a bit. I don't > > think I've seen a more specific helper for this situation. > > Hmm, not super appropriate. How about something ala: > > if (buf_addr + len < buf_addr) > ... overflow ... > > ? Sure, sounds good.
