diff options
| author | Pavel Begunkov <asml.silence@gmail.com> | 2025-02-26 11:41:17 +0000 |
|---|---|---|
| committer | Jens Axboe <axboe@kernel.dk> | 2025-02-27 07:27:55 -0700 |
| commit | a223e96f7305fc482b4b41424e53d12ef693793d (patch) | |
| tree | 95c29125da0e054b844e678c4f32275a38dfd767 | |
| parent | 0fc5a589aff7f9e613ff94fd3dd0a6686ffcb706 (diff) | |
io_uring/net: isolate msghdr copying code
The user access section in io_msg_copy_hdr() is overextended by covering
selected buffers. It's hard to work with and prone to errors. Limit the
section to msghdr import only, selected buffers will do a separate
copy_from_user() call, and then move it into its own function. This
should be fine, selected buffer single shots are not important, for
multishots the overhead should be non-existent, and it's not that
expensive overall.
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
Link: https://lore.kernel.org/r/d3eb1f81c8cfbea9f1aa57dab90c472d2aa6e371.1740569495.git.asml.silence@gmail.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
| -rw-r--r-- | io_uring/net.c | 45 |
1 files changed, 25 insertions, 20 deletions
diff --git a/io_uring/net.c b/io_uring/net.c index dc1569864799..1e176f48c268 100644 --- a/io_uring/net.c +++ b/io_uring/net.c @@ -241,6 +241,24 @@ static int io_compat_msg_copy_hdr(struct io_kiocb *req, } #endif +static int io_copy_msghdr_from_user(struct user_msghdr *msg, + struct user_msghdr __user *umsg) +{ + if (!user_access_begin(umsg, sizeof(*umsg))) + return -EFAULT; + unsafe_get_user(msg->msg_name, &umsg->msg_name, ua_end); + unsafe_get_user(msg->msg_namelen, &umsg->msg_namelen, ua_end); + unsafe_get_user(msg->msg_iov, &umsg->msg_iov, ua_end); + unsafe_get_user(msg->msg_iovlen, &umsg->msg_iovlen, ua_end); + unsafe_get_user(msg->msg_control, &umsg->msg_control, ua_end); + unsafe_get_user(msg->msg_controllen, &umsg->msg_controllen, ua_end); + user_access_end(); + return 0; +ua_end: + user_access_end(); + return -EFAULT; +} + static int io_msg_copy_hdr(struct io_kiocb *req, struct io_async_msghdr *iomsg, struct user_msghdr *msg, int ddir) { @@ -257,16 +275,10 @@ static int io_msg_copy_hdr(struct io_kiocb *req, struct io_async_msghdr *iomsg, nr_segs = 1; } - if (!user_access_begin(umsg, sizeof(*umsg))) - return -EFAULT; + ret = io_copy_msghdr_from_user(msg, umsg); + if (unlikely(ret)) + return ret; - ret = -EFAULT; - unsafe_get_user(msg->msg_name, &umsg->msg_name, ua_end); - unsafe_get_user(msg->msg_namelen, &umsg->msg_namelen, ua_end); - unsafe_get_user(msg->msg_iov, &umsg->msg_iov, ua_end); - unsafe_get_user(msg->msg_iovlen, &umsg->msg_iovlen, ua_end); - unsafe_get_user(msg->msg_control, &umsg->msg_control, ua_end); - unsafe_get_user(msg->msg_controllen, &umsg->msg_controllen, ua_end); msg->msg_flags = 0; if (req->flags & REQ_F_BUFFER_SELECT) { @@ -274,24 +286,17 @@ static int io_msg_copy_hdr(struct io_kiocb *req, struct io_async_msghdr *iomsg, sr->len = iov->iov_len = 0; iov->iov_base = NULL; } else if (msg->msg_iovlen > 1) { - ret = -EINVAL; - goto ua_end; + return -EINVAL; } else { struct iovec __user *uiov = msg->msg_iov; - /* we only need the length for provided buffers */ - if (!access_ok(&uiov->iov_len, sizeof(uiov->iov_len))) - goto ua_end; - unsafe_get_user(iov->iov_len, &uiov->iov_len, ua_end); + if (copy_from_user(iov, uiov, sizeof(*iov))) + return -EFAULT; sr->len = iov->iov_len; } - ret = 0; -ua_end: - user_access_end(); - return ret; + return 0; } - user_access_end(); ret = __import_iovec(ddir, msg->msg_iov, msg->msg_iovlen, nr_segs, &iov, &iomsg->msg.msg_iter, false); if (unlikely(ret < 0)) |