| Message ID | x497ef4hzuj.fsf@segfault.boston.devel.redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | io_uring: don't allow duplicate registrations | expand |
On 1/16/19 5:50 PM, Jeff Moyer wrote: > Hi, Jens, > > It looks to me like calling io_uring_register more than once (for either > IORING_REGISTER_BUFFERS or IORING_REGISTER_FILES) will leak the > references taken in previous calls. Oops, thanks for that. Let's make it -EBUSY though, everything ends up being -EINVAL and that sometimes makes it hard for an application to figure out wtf is going on. I also added -EINVAL for unregister when not already setup.
Jens Axboe <axboe@kernel.dk> writes: > On 1/16/19 5:50 PM, Jeff Moyer wrote: >> Hi, Jens, >> >> It looks to me like calling io_uring_register more than once (for either >> IORING_REGISTER_BUFFERS or IORING_REGISTER_FILES) will leak the >> references taken in previous calls. > > Oops, thanks for that. Let's make it -EBUSY though, everything ends up > being -EINVAL and that sometimes makes it hard for an application to > figure out wtf is going on. Sounds good. > I also added -EINVAL for unregister when not already setup. Yep, I saw that. -Jeff
On 1/16/19 6:31 PM, Jeff Moyer wrote: > Jens Axboe <axboe@kernel.dk> writes: > >> On 1/16/19 5:50 PM, Jeff Moyer wrote: >>> Hi, Jens, >>> >>> It looks to me like calling io_uring_register more than once (for either >>> IORING_REGISTER_BUFFERS or IORING_REGISTER_FILES) will leak the >>> references taken in previous calls. >> >> Oops, thanks for that. Let's make it -EBUSY though, everything ends up >> being -EINVAL and that sometimes makes it hard for an application to >> figure out wtf is going on. > > Sounds good. > >> I also added -EINVAL for unregister when not already setup. > > Yep, I saw that. Changed it to -ENXIO, that's more descriptive.
diff --git a/fs/io_uring.c b/fs/io_uring.c index 3650e8b63a32..f0492b0e23a0 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1399,6 +1399,9 @@ static int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg, if (!nr_args) return -EINVAL; + if (ctx->user_files) + return -EINVAL; + ctx->user_files = kcalloc(nr_args, sizeof(struct file *), GFP_KERNEL); if (!ctx->user_files) return -ENOMEM; @@ -1580,6 +1583,9 @@ static int io_sqe_buffer_register(struct io_ring_ctx *ctx, void __user *arg, if (!nr_args || nr_args > UIO_MAXIOV) return -EINVAL; + if (ctx->user_bufs) + return -EINVAL; + ctx->user_bufs = kcalloc(nr_args, sizeof(struct io_mapped_ubuf), GFP_KERNEL); if (!ctx->user_bufs)
Hi, Jens, It looks to me like calling io_uring_register more than once (for either IORING_REGISTER_BUFFERS or IORING_REGISTER_FILES) will leak the references taken in previous calls. Signed-off-by: Jeff Moyer <jmoyer@redhat.com> --- If this makes sense to you, feel free to just fold this into your patches w/o any attribution.