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
