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

[Pavel Begunkov <asml.silence@gmail.com>]

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.

-- 
Pavel Begunkov