On 6/20/23 2:26?AM, Stefan Metzmacher wrote: > Hi Jens, > >> We cannot sanely handle partial retries for recvmsg if we have cmsg >> attached. If we don't, then we'd just be overwriting the initial cmsg >> header on retries. Alternatively we could increment and handle this >> appropriately, but it doesn't seem worth the complication. >> >> Link: https://lore.kernel.org/io-uring/0b0d4411-c8fd-4272-770b-e030af6919a0@xxxxxxxxx/ >> Cc: stable@xxxxxxxxxxxxxxx # 5.10+ >> Reported-by: Stefan Metzmacher <metze@xxxxxxxxx> >> Signed-off-by: Jens Axboe <axboe@xxxxxxxxx> >> >> --- >> >> diff --git a/io_uring/net.c b/io_uring/net.c >> index fe1c478c7dec..6674a0759390 100644 >> --- a/io_uring/net.c >> +++ b/io_uring/net.c >> @@ -788,7 +788,8 @@ int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags) >> flags = sr->msg_flags; >> if (force_nonblock) >> flags |= MSG_DONTWAIT; >> - if (flags & MSG_WAITALL) >> + /* disable partial retry for recvmsg with cmsg attached */ >> + if (flags & MSG_WAITALL && !kmsg->controllen) >> min_ret = iov_iter_count(&kmsg->msg.msg_iter); > > Isn't kmsg->controllen only used for REQ_F_APOLL_MULTISHOT? > > I guess the correct value would be kmsg->msg.msg_controllen? Gah, yes indeed it should! > Maybe the safest change would be something like this (completely untested!): > > diff --git a/io_uring/net.c b/io_uring/net.c > index 89e839013837..1dd5322fb732 100644 > --- a/io_uring/net.c > +++ b/io_uring/net.c > @@ -781,14 +781,14 @@ int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags) > flags = sr->msg_flags; > if (force_nonblock) > flags |= MSG_DONTWAIT; > - if (flags & MSG_WAITALL) > - min_ret = iov_iter_count(&kmsg->msg.msg_iter); > > kmsg->msg.msg_get_inq = 1; > if (req->flags & REQ_F_APOLL_MULTISHOT) > ret = io_recvmsg_multishot(sock, sr, kmsg, flags, > &mshot_finished); > else > + if (flags & MSG_WAITALL && !kmsg->msg.msg_controllen) > + min_ret = iov_iter_count(&kmsg->msg.msg_iter); > ret = __sys_recvmsg_sock(sock, &kmsg->msg, sr->umsg, > kmsg->uaddr, flags); I like putting it there, as MSG_WAITALL is explicitly disallowed for multishot anyway. The check belongs in that branch rather than as a whole. -- Jens Axboe
