[PATCH] io_uring: check file_slot early when accept use fix_file mode

[PATCH] io_uring: check file_slot early when accept use fix_file mode

[Hao Xu]

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

Re: [PATCH] io_uring: check file_slot early when accept use fix_file mode

[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.

-- 
Jens Axboe

Re: [PATCH] io_uring: check file_slot early when accept use fix_file mode

[Hao Xu]

在 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?
> 

Re: [PATCH] io_uring: check file_slot early when accept use fix_file mode

[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.

-- 
Jens Axboe

Re: [PATCH] io_uring: check file_slot early when accept use fix_file mode

[Hao Xu]

在 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.
> 

Re: [PATCH] io_uring: check file_slot early when accept use fix_file mode

[Jens Axboe]

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