Re: [PATCH 8/8] io_uring/net: set MSG_MORE if we're doing multishot send and have more

[Dylan Yudaken <dyudaken@gmail.com>]

On Sun, Feb 25, 2024 at 12:46 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> If we have more data pending, we know we're going to do one more loop.
> If that's the case, then set MSG_MORE to inform the networking stack
> that there's more data coming shortly for this socket.
>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
>  io_uring/net.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/io_uring/net.c b/io_uring/net.c
> index 240b8eff1a78..07307dd5a077 100644
> --- a/io_uring/net.c
> +++ b/io_uring/net.c
> @@ -519,6 +519,10 @@ int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
>         if (!io_check_multishot(req, issue_flags))
>                 return io_setup_async_msg(req, kmsg, issue_flags);
>
> +       flags = sr->msg_flags;
> +       if (issue_flags & IO_URING_F_NONBLOCK)
> +               flags |= MSG_DONTWAIT;
> +
>  retry_multishot:
>         if (io_do_buffer_select(req)) {
>                 void __user *buf;
> @@ -528,12 +532,12 @@ int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
>                 if (!buf)
>                         return -ENOBUFS;
>
> +               if ((req->flags & (REQ_F_BL_EMPTY|REQ_F_APOLL_MULTISHOT)) ==
> +                                  REQ_F_APOLL_MULTISHOT)
> +                       flags |= MSG_MORE;
>                 iov_iter_ubuf(&kmsg->msg.msg_iter, ITER_SOURCE, buf, len);
>         }

This feels racy. I don't have an exact sequence in mind, but I believe
there are cases where between
the two calls to __sys_sendmsg_sock, another submission could be
issued and drain the buffer list.
I guess the result would be that the packet is never sent out, but I
have not followed the codepaths of MSG_MORE.

The obvious other way to trigger this codepath is if the user messes
with the ring by decrementing
the buffer counter. I do not believe there are any nefarious outcomes
- but just to point out that
REQ_F_BL_EMPTY is essentially user controlled.