On 5/27/22 10:53 AM, Xiaoguang Wang wrote:
> io_fixed_fd_install() may fail for many reasons, such as short of
> free fixed file bitmap, memory allocation failures, etc. When these
> errors happen, current code forgets to fput(file) correspondingly.
>
> This patch will fix resource leaks around io_fixed_fd_install(),
> meanwhile io_fixed_fd_install() and io_install_fixed_file() are
> basically similar, fold them into one function.
>
> Signed-off-by: Xiaoguang Wang <xiaoguang.wang@xxxxxxxxxxxxxxxxx>
> ---
> fs/io_uring.c | 77 ++++++++++++++++++++++++++---------------------------------
> 1 file changed, 34 insertions(+), 43 deletions(-)
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index d50bbf8de4fb..ff50e5f1753d 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -1364,8 +1364,8 @@ static void io_req_task_queue(struct io_kiocb *req);
> static void __io_submit_flush_completions(struct io_ring_ctx *ctx);
> static int io_req_prep_async(struct io_kiocb *req);
>
> -static int io_install_fixed_file(struct io_kiocb *req, struct file *file,
> - unsigned int issue_flags, u32 slot_index);
> +static int io_install_fixed_file(struct io_kiocb *req, unsigned int issue_flags,
> + struct file *file, u32 slot);
> static int io_close_fixed(struct io_kiocb *req, unsigned int issue_flags);
>
> static enum hrtimer_restart io_link_timeout_fn(struct hrtimer *timer);
> @@ -5438,36 +5438,6 @@ static int io_file_bitmap_get(struct io_ring_ctx *ctx)
> return -ENFILE;
> }
>
> -static int io_fixed_fd_install(struct io_kiocb *req, unsigned int issue_flags,
> - struct file *file, unsigned int file_slot)
> -{
> - bool alloc_slot = file_slot == IORING_FILE_INDEX_ALLOC;
> - struct io_ring_ctx *ctx = req->ctx;
> - int ret;
> -
> - if (alloc_slot) {
> - io_ring_submit_lock(ctx, issue_flags);
> - ret = io_file_bitmap_get(ctx);
> - if (unlikely(ret < 0)) {
> - io_ring_submit_unlock(ctx, issue_flags);
> - return ret;
> - }
> -
> - file_slot = ret;
> - } else {
> - file_slot--;
> - }
> -
> - ret = io_install_fixed_file(req, file, issue_flags, file_slot);
> - if (alloc_slot) {
> - io_ring_submit_unlock(ctx, issue_flags);
> - if (!ret)
> - return file_slot;
> - }
> -
> - return ret;
> -}
> -
> static int io_openat2(struct io_kiocb *req, unsigned int issue_flags)
> {
> struct open_flags op;
> @@ -5520,11 +5490,14 @@ static int io_openat2(struct io_kiocb *req, unsigned int issue_flags)
> file->f_flags &= ~O_NONBLOCK;
> fsnotify_open(file);
>
> - if (!fixed)
> + if (!fixed) {
> fd_install(ret, file);
> - else
> - ret = io_fixed_fd_install(req, issue_flags, file,
> - req->open.file_slot);
> + } else {
> + ret = io_install_fixed_file(req, issue_flags, file,
> + req->open.file_slot);
> + if (ret < 0)
> + fput(file);
> + }
> err:
> putname(req->open.filename);
> req->flags &= ~REQ_F_NEED_CLEANUP;
> @@ -6603,8 +6576,10 @@ static int io_accept(struct io_kiocb *req, unsigned int issue_flags)
> fd_install(fd, file);
> ret = fd;
> } else {
> - ret = io_fixed_fd_install(req, issue_flags, file,
> - accept->file_slot);
> + ret = io_install_fixed_file(req, issue_flags, file,
> + accept->file_slot);
> + if (ret < 0)
> + fput(file);
> }
>
> if (!(req->flags & REQ_F_APOLL_MULTISHOT)) {
> @@ -6676,8 +6651,10 @@ static int io_socket(struct io_kiocb *req, unsigned int issue_flags)
> fd_install(fd, file);
> ret = fd;
> } else {
> - ret = io_fixed_fd_install(req, issue_flags, file,
> + ret = io_install_fixed_file(req, issue_flags, file,
> sock->file_slot);
> + if (ret < 0)
> + fput(file);
> }
> __io_req_complete(req, issue_flags, ret, 0);
> return 0;
> @@ -10130,15 +10107,27 @@ static int io_queue_rsrc_removal(struct io_rsrc_data *data, unsigned idx,
> return 0;
> }
>
> -static int io_install_fixed_file(struct io_kiocb *req, struct file *file,
> - unsigned int issue_flags, u32 slot_index)
> +static int io_install_fixed_file(struct io_kiocb *req, unsigned int issue_flags,
> + struct file *file, u32 slot)
> {
> struct io_ring_ctx *ctx = req->ctx;
> bool needs_switch = false;
> struct io_fixed_file *file_slot;
> int ret = -EBADF;
> + bool alloc_slot = slot == IORING_FILE_INDEX_ALLOC;
> + int slot_index;
>
> io_ring_submit_lock(ctx, issue_flags);
> + if (alloc_slot) {
> + slot_index = io_file_bitmap_get(ctx);
> + if (unlikely(slot_index < 0)) {
> + io_ring_submit_unlock(ctx, issue_flags);
> + return slot_index;
> + }
> + } else {
> + slot_index = slot - 1;
> + }
> +
> if (file->f_op == &io_uring_fops)
> goto err;
> ret = -ENXIO;
> @@ -10178,8 +10167,10 @@ static int io_install_fixed_file(struct io_kiocb *req, struct file *file,
> if (needs_switch)
> io_rsrc_node_switch(ctx, ctx->file_data);
> io_ring_submit_unlock(ctx, issue_flags);
> - if (ret)
> - fput(file);
> + if (alloc_slot) {
> + if (!ret)
> + return slot_index;
> + }
> return ret;
> }
>
--
Jens Axboe
