public inbox for [email protected]
 help / color / mirror / Atom feed
From: Jens Axboe <[email protected]>
To: Xiaoguang Wang <[email protected]>,
	[email protected]
Cc: [email protected]
Subject: Re: [PATCH 1/2] io_uring: fix file leaks around io_fixed_fd_install()
Date: Fri, 27 May 2022 11:00:37 -0600	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

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 <[email protected]>
> ---
>  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


  reply	other threads:[~2022-05-27 17:00 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-27 16:53 [PATCH 0/2] two bug fixes about direct fd install Xiaoguang Wang
2022-05-27 16:53 ` [PATCH 1/2] io_uring: fix file leaks around io_fixed_fd_install() Xiaoguang Wang
2022-05-27 17:00   ` Jens Axboe [this message]
2022-05-27 16:53 ` [PATCH 2/2] io_uring: defer alloc_hint update to io_file_bitmap_set() Xiaoguang Wang
2022-05-27 17:09   ` Jens Axboe
2022-05-27 17:39     ` [PATCH v2] " Xiaoguang Wang
2022-05-27 18:03       ` Jens Axboe
2022-05-28  1:45         ` Xiaoguang Wang
2022-05-28  1:51         ` [PATCH v3] " Xiaoguang Wang
2022-05-28  2:14           ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox