io-uring.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] io_uring: check file_slot early when accept use fix_file mode
@ 2021-09-07 15:16 Hao Xu
  2021-09-07 15:24 ` Jens Axboe
  0 siblings, 1 reply; 6+ messages in thread
From: Hao Xu @ 2021-09-07 15:16 UTC (permalink / raw)
  To: Jens Axboe; +Cc: io-uring, Pavel Begunkov, Joseph Qi

check file_slot early in io_accept_prep() to avoid wasted effort in
failure cases.

Signed-off-by: Hao Xu <haoxu@linux.alibaba.com>
---
 fs/io_uring.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 30d959416eba..917271bd80c5 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -4868,6 +4868,8 @@ static int io_accept_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	accept->nofile = rlimit(RLIMIT_NOFILE);
 
 	accept->file_slot = READ_ONCE(sqe->file_index);
+	if (accept->file_slot - 1 >= req->ctx->nr_user_files)
+		return -EINVAL;
 	if (accept->file_slot && ((req->open.how.flags & O_CLOEXEC) ||
 				  (accept->flags & SOCK_CLOEXEC)))
 		return -EINVAL;
-- 
2.24.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] io_uring: check file_slot early when accept use fix_file mode
  2021-09-07 15:16 [PATCH] io_uring: check file_slot early when accept use fix_file mode Hao Xu
@ 2021-09-07 15:24 ` Jens Axboe
  2021-09-07 15:32   ` Hao Xu
  0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2021-09-07 15:24 UTC (permalink / raw)
  To: Hao Xu; +Cc: io-uring, Pavel Begunkov, Joseph Qi

On 9/7/21 9:16 AM, Hao Xu wrote:
> check file_slot early in io_accept_prep() to avoid wasted effort in
> failure cases.

It's generally better to just let the failure cases deal with it instead
of having checks in multiple places. This is a failure path, so we don't
care about making it fail early. Optimizations should be for the hot path,
which is not a malformed sqe.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] io_uring: check file_slot early when accept use fix_file mode
  2021-09-07 15:24 ` Jens Axboe
@ 2021-09-07 15:32   ` Hao Xu
  2021-09-07 15:37     ` Jens Axboe
  0 siblings, 1 reply; 6+ messages in thread
From: Hao Xu @ 2021-09-07 15:32 UTC (permalink / raw)
  To: Jens Axboe; +Cc: io-uring, Pavel Begunkov, Joseph Qi

在 2021/9/7 下午11:24, Jens Axboe 写道:
> On 9/7/21 9:16 AM, Hao Xu wrote:
>> check file_slot early in io_accept_prep() to avoid wasted effort in
>> failure cases.
> 
> It's generally better to just let the failure cases deal with it instead
> of having checks in multiple places. This is a failure path, so we don't
> care about making it fail early. Optimizations should be for the hot path,
> which is not a malformed sqe.
I have a question here: if we do do_accept() and but fail in
io_install_fixed_file(), do we lose the conn_fd return by do_accept()
forever?
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] io_uring: check file_slot early when accept use fix_file mode
  2021-09-07 15:32   ` Hao Xu
@ 2021-09-07 15:37     ` Jens Axboe
  2021-09-07 15:42       ` Hao Xu
  0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2021-09-07 15:37 UTC (permalink / raw)
  To: Hao Xu; +Cc: io-uring, Pavel Begunkov, Joseph Qi

On 9/7/21 9:32 AM, Hao Xu wrote:
> 在 2021/9/7 下午11:24, Jens Axboe 写道:
>> On 9/7/21 9:16 AM, Hao Xu wrote:
>>> check file_slot early in io_accept_prep() to avoid wasted effort in
>>> failure cases.
>>
>> It's generally better to just let the failure cases deal with it instead
>> of having checks in multiple places. This is a failure path, so we don't
>> care about making it fail early. Optimizations should be for the hot path,
>> which is not a malformed sqe.
> I have a question here: if we do do_accept() and but fail in
> io_install_fixed_file(), do we lose the conn_fd return by do_accept()
> forever?

We do. The file is put and everything, so we're not leaking anything.
But the actual connection is lost as the accept request failed.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] io_uring: check file_slot early when accept use fix_file mode
  2021-09-07 15:37     ` Jens Axboe
@ 2021-09-07 15:42       ` Hao Xu
  2021-09-07 15:44         ` Jens Axboe
  0 siblings, 1 reply; 6+ messages in thread
From: Hao Xu @ 2021-09-07 15:42 UTC (permalink / raw)
  To: Jens Axboe; +Cc: io-uring, Pavel Begunkov, Joseph Qi

在 2021/9/7 下午11:37, Jens Axboe 写道:
> On 9/7/21 9:32 AM, Hao Xu wrote:
>> 在 2021/9/7 下午11:24, Jens Axboe 写道:
>>> On 9/7/21 9:16 AM, Hao Xu wrote:
>>>> check file_slot early in io_accept_prep() to avoid wasted effort in
>>>> failure cases.
>>>
>>> It's generally better to just let the failure cases deal with it instead
>>> of having checks in multiple places. This is a failure path, so we don't
>>> care about making it fail early. Optimizations should be for the hot path,
>>> which is not a malformed sqe.
>> I have a question here: if we do do_accept() and but fail in
>> io_install_fixed_file(), do we lose the conn_fd return by do_accept()
>> forever?
> 
> We do. The file is put and everything, so we're not leaking anything.
> But the actual connection is lost as the accept request failed.
Does that cause any problem, since from client's perspective, connection
is builded, and there is no way for the server to close it.
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] io_uring: check file_slot early when accept use fix_file mode
  2021-09-07 15:42       ` Hao Xu
@ 2021-09-07 15:44         ` Jens Axboe
  0 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2021-09-07 15:44 UTC (permalink / raw)
  To: Hao Xu; +Cc: io-uring, Pavel Begunkov, Joseph Qi

On 9/7/21 9:42 AM, Hao Xu wrote:
> 在 2021/9/7 下午11:37, Jens Axboe 写道:
>> On 9/7/21 9:32 AM, Hao Xu wrote:
>>> 在 2021/9/7 下午11:24, Jens Axboe 写道:
>>>> On 9/7/21 9:16 AM, Hao Xu wrote:
>>>>> check file_slot early in io_accept_prep() to avoid wasted effort in
>>>>> failure cases.
>>>>
>>>> It's generally better to just let the failure cases deal with it instead
>>>> of having checks in multiple places. This is a failure path, so we don't
>>>> care about making it fail early. Optimizations should be for the hot path,
>>>> which is not a malformed sqe.
>>> I have a question here: if we do do_accept() and but fail in
>>> io_install_fixed_file(), do we lose the conn_fd return by do_accept()
>>> forever?
>>
>> We do. The file is put and everything, so we're not leaking anything.
>> But the actual connection is lost as the accept request failed.
> Does that cause any problem, since from client's perspective, connection
> is builded, and there is no way for the server to close it.

Maybe? But it's a program error, it's asking to accept a connection at a
slot which is invalid. It kind of gets to keep the pieces in that
particular case.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-09-07 15:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-07 15:16 [PATCH] io_uring: check file_slot early when accept use fix_file mode Hao Xu
2021-09-07 15:24 ` Jens Axboe
2021-09-07 15:32   ` Hao Xu
2021-09-07 15:37     ` Jens Axboe
2021-09-07 15:42       ` Hao Xu
2021-09-07 15:44         ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).