Re: [PATCH] io_uring/net: fix a multishot termination case for recv

[Jens Axboe <axboe@kernel.dk>]

On 9/29/24 1:25 PM, Pavel Begunkov wrote:
> On 9/28/24 13:40, Jens Axboe wrote:
>> On 9/28/24 6:18 AM, Jens Axboe wrote:
>>> diff --git a/io_uring/net.c b/io_uring/net.c
>>> index f10f5a22d66a..18507658a921 100644
>>> --- a/io_uring/net.c
>>> +++ b/io_uring/net.c
>>> @@ -1133,6 +1133,7 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
>>>       int ret, min_ret = 0;
>>>       bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
>>>       size_t len = sr->len;
>>> +    bool mshot_finished;
>>>         if (!(req->flags & REQ_F_POLLED) &&
>>>           (sr->flags & IORING_RECVSEND_POLL_FIRST))
>>> @@ -1187,6 +1188,7 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
>>>           req_set_fail(req);
>>>       }
>>>   +    mshot_finished = ret <= 0;
>>>       if (ret > 0)
>>>           ret += sr->done_io;
>>>       else if (sr->done_io)
>>> @@ -1194,7 +1196,7 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
>>>       else
>>>           io_kbuf_recycle(req, issue_flags);
>>>   -    if (!io_recv_finish(req, &ret, kmsg, ret <= 0, issue_flags))
>>> +    if (!io_recv_finish(req, &ret, kmsg, mshot_finished, issue_flags))
>>>           goto retry_multishot;
>>>         return ret;
>>
>> On second thought, I don't think we can get into this situation -
>> sr->done_io is only ever used for recv if we had to retry after getting
>> some data. And that only happens if MSG_WAITALL is set, which is not
>> legal for multishot and will result in -EINVAL. So don't quite see how
>> we can run into this issue. But I could be missing something...
>>
>> Comments?
> 
> I noticed the chunk months ago, it's definitely a sloppy one, but I
> deemed it not to be an actual problem after trying to exploit it at
> the moment.

Yes, might not be a bad idea to still do the tweak. Not because we can
_currently_ hit it, but because it could be possible in the future and
just get overlooked.

Thanks for confirming :-)

-- 
Jens Axboe