diff --git a/LibOS/shim/src/sys/shim_socket.c b/LibOS/shim/src/sys/shim_socket.c index 9c70a3fe..16ac0c62 100644 --- a/LibOS/shim/src/sys/shim_socket.c +++ b/LibOS/shim/src/sys/shim_socket.c @@ -990,20 +990,6 @@ static ssize_t do_sendmsg(int fd, struct iovec* bufs, int nbufs, int flags, if (hdl->type != TYPE_SOCK) goto out; - struct shim_sock_handle* sock = &hdl->info.sock; - - ret = -EFAULT; - if (addr && test_user_memory((void*)addr, addrlen, false)) - goto out; - - if (!bufs || test_user_memory(bufs, sizeof(*bufs) * nbufs, false)) - goto out; - - for (int i = 0; i < nbufs; i++) { - if (!bufs[i].iov_base || test_user_memory(bufs[i].iov_base, bufs[i].iov_len, false)) - goto out; - } - if (flags & ~(MSG_NOSIGNAL | MSG_DONTWAIT)) { debug("sendmsg()/sendmmsg()/sendto(): unknown flag (only MSG_NOSIGNAL and MSG_DONTWAIT" " are supported).\n"); @@ -1011,6 +997,15 @@ static ssize_t do_sendmsg(int fd, struct iovec* bufs, int nbufs, int flags, goto out; } + struct shim_sock_handle* sock = &hdl->info.sock; + + if (addr) { + if (addrlen < 0 || (size_t)addrlen < minimal_addrlen(sock->domain)) { + ret = -EINVAL; + goto out; + } + } + lock(&hdl->lock); if (flags & MSG_DONTWAIT) { @@ -1130,6 +1125,14 @@ out: ssize_t shim_do_sendto(int sockfd, const void* buf, size_t len, int flags, const struct sockaddr* addr, int addrlen) { + if (addr && test_user_memory((void*)addr, addrlen, /*write=*/false)) { + return -EFAULT; + } + + if (!buf || test_user_memory((void*)buf, len, /*write=*/false)) { + return -EFAULT; + } + struct iovec iovbuf; iovbuf.iov_base = (void*)buf; iovbuf.iov_len = len; @@ -1137,17 +1140,63 @@ ssize_t shim_do_sendto(int sockfd, const void* buf, size_t len, int flags, return do_sendmsg(sockfd, &iovbuf, 1, flags, addr, addrlen); } +static int check_msghdr(struct msghdr* msg, bool is_recv) { + if (msg->msg_namelen < 0) { + return -EINVAL; + } + + if (test_user_memory(msg->msg_name, msg->msg_namelen, /*write=*/is_recv)) { + return -EFAULT; + } + + size_t size; + if (__builtin_mul_overflow(sizeof(*msg->msg_iov), msg->msg_iovlen, &size)) { + return -EMSGSIZE; + } + + if (test_user_memory(msg->msg_iov, size, /*write=*/false)) { + return -EFAULT; + } + + struct iovec* bufs = msg->msg_iov; + for (size_t i = 0; i < msg->msg_iovlen; i++) { + if (test_user_memory(bufs[i].iov_base, bufs[i].iov_len, /*write=*/is_recv)) { + return -EFAULT; + } + } + + return 0; +} + ssize_t shim_do_sendmsg(int sockfd, struct msghdr* msg, int flags) { + if (!msg || test_user_memory(msg, sizeof(*msg), /*write=*/false)) { + return -EFAULT; + } + + int ret = check_msghdr(msg, /*is_recv=*/false); + if (ret < 0) { + return ret; + } + return do_sendmsg(sockfd, msg->msg_iov, msg->msg_iovlen, flags, msg->msg_name, msg->msg_namelen); } ssize_t shim_do_sendmmsg(int sockfd, struct mmsghdr* msg, unsigned int vlen, int flags) { - if (test_user_memory(msg, vlen, /*write=*/true)) + if (test_user_memory(msg, sizeof(*msg) * vlen, /*write=*/true)) { return -EFAULT; + } + for (size_t i = 0; i < vlen; i++) { + struct msghdr* m = &msg[i].msg_hdr; + + int ret = check_msghdr(m, /*is_recv=*/false); + if (ret < 0) { + return ret; + } + } ssize_t total = 0; - for (size_t i = 0; i * sizeof(struct mmsghdr) < vlen; i++) { + for (size_t i = 0; i < vlen; i++) { struct msghdr* m = &msg[i].msg_hdr; ssize_t bytes = @@ -1178,32 +1227,14 @@ static ssize_t do_recvmsg(int fd, struct iovec* bufs, size_t nbufs, int flags, struct shim_sock_handle* sock = &hdl->info.sock; if (addr) { - ret = -EINVAL; - if (!addrlen || test_user_memory(addrlen, sizeof(*addrlen), /*write=*/true)) - goto out; - - if (*addrlen < 0 || (size_t)*addrlen < minimal_addrlen(sock->domain)) - goto out; - - if (test_user_memory(addr, *addrlen, /*write=*/true)) + if (*addrlen < 0 || (size_t)*addrlen < minimal_addrlen(sock->domain)) { + ret = -EINVAL; goto out; + } } - size_t bufs_size; - if (__builtin_mul_overflow(sizeof(*bufs), nbufs, &bufs_size)) { - ret = -EMSGSIZE; - goto out; - } - - ret = -EFAULT; - if (!bufs || test_user_memory(bufs, bufs_size, /*write=*/false)) - goto out; - size_t expected_size = 0; for (size_t i = 0; i < nbufs; i++) { - if (!bufs[i].iov_base || test_user_memory(bufs[i].iov_base, bufs[i].iov_len, - /*write=*/true)) - goto out; expected_size += bufs[i].iov_len; } @@ -1413,6 +1444,24 @@ out: ssize_t shim_do_recvfrom(int sockfd, void* buf, size_t len, int flags, struct sockaddr* addr, int* addrlen) { + if (addr) { + if (test_user_memory(addrlen, sizeof(*addrlen), /*write=*/true)) { + return -EFAULT; + } + + if (*addrlen < 0) { + return -EINVAL; + } + + if (test_user_memory(addr, *addrlen, /*write=*/true)) { + return -EFAULT; + } + } + + if (test_user_memory(buf, len, /*write=*/true)) { + return -EFAULT; + } + struct iovec iovbuf; iovbuf.iov_base = (void*)buf; iovbuf.iov_len = len; @@ -1421,16 +1470,33 @@ ssize_t shim_do_recvfrom(int sockfd, void* buf, size_t len, int flags, struct so } ssize_t shim_do_recvmsg(int sockfd, struct msghdr* msg, int flags) { + if (test_user_memory(msg, sizeof(*msg), /*write=*/true)) { + return -EFAULT; + } + + int ret = check_msghdr(msg, /*is_recv=*/true); + if (ret < 0) { + return ret; + } + return do_recvmsg(sockfd, msg->msg_iov, msg->msg_iovlen, flags, msg->msg_name, &msg->msg_namelen); } ssize_t shim_do_recvmmsg(int sockfd, struct mmsghdr* msg, unsigned int vlen, int flags, struct __kernel_timespec* timeout) { - if (test_user_memory(msg, vlen, /*write=*/true)) + if (test_user_memory(msg, sizeof(*msg) * vlen, /*write=*/true)) return -EFAULT; - ssize_t total = 0; + for (size_t i = 0; i < vlen; i++) { + struct msghdr* m = &msg[i].msg_hdr; + + int ret = check_msghdr(m, /*is_recv=*/true); + if (ret < 0) { + return ret; + } + } + // Issue # 753 - https://github.com/oscarlab/graphene/issues/753 /* TODO(donporter): timeout properly. For now, explicitly return an error. */ if (timeout) { @@ -1438,7 +1504,8 @@ ssize_t shim_do_recvmmsg(int sockfd, struct mmsghdr* msg, unsigned int vlen, int return -EOPNOTSUPP; } - for (size_t i = 0; i * sizeof(struct mmsghdr) < vlen; i++) { + ssize_t total = 0; + for (size_t i = 0; i < vlen; i++) { struct msghdr* m = &msg[i].msg_hdr; ssize_t bytes = diff --git a/LibOS/shim/src/sys/shim_wait.c b/LibOS/shim/src/sys/shim_wait.c index 2aa5bde0..1975c162 100644 --- a/LibOS/shim/src/sys/shim_wait.c +++ b/LibOS/shim/src/sys/shim_wait.c @@ -93,20 +93,11 @@ static void remove_qnode_from_wait_queue(struct shim_thread_queue* qnode) { } } -long shim_do_waitid(int which, pid_t id, siginfo_t* infop, int options, struct __kernel_rusage* ru) { - __UNUSED(ru); - - if (options & ~(WNOHANG | WNOWAIT | WEXITED | WSTOPPED | WCONTINUED | - __WNOTHREAD | __WCLONE | __WALL)) - return -EINVAL; - +static long do_waitid(int which, pid_t id, siginfo_t* infop, int options) { if (options & __WALL) { options &= ~__WCLONE; } - if (!(options & (WEXITED | WSTOPPED | WCONTINUED))) - return -EINVAL; - if (options & WSTOPPED) { debug("Ignoring unsupported WSTOPPED flag to wait4\n"); options &= ~WSTOPPED; @@ -125,9 +116,6 @@ long shim_do_waitid(int which, pid_t id, siginfo_t* infop, int options, struct _ if (!(which == P_PGID || which == P_ALL || which == P_PID)) return -EINVAL; - if (infop && test_user_memory(infop, sizeof(*infop), /*write=*/true)) - return -EFAULT; - long ret = 0; lock(&g_process.children_lock); @@ -225,7 +213,25 @@ out: return ret; } +long shim_do_waitid(int which, pid_t id, siginfo_t* infop, int options, struct __kernel_rusage* ru) { + __UNUSED(ru); + + if (options & ~(WNOHANG | WNOWAIT | WEXITED | WSTOPPED | WCONTINUED | + __WNOTHREAD | __WCLONE | __WALL)) + return -EINVAL; + + if (!(options & (WEXITED | WSTOPPED | WCONTINUED))) + return -EINVAL; + + if (infop && test_user_memory(infop, sizeof(*infop), /*write=*/true)) + return -EFAULT; + + return do_waitid(which, id, infop, options); +} + long shim_do_wait4(pid_t pid, int* status, int options, struct __kernel_rusage* ru) { + __UNUSED(ru); + int which; pid_t id; siginfo_t info; @@ -237,7 +243,7 @@ long shim_do_wait4(pid_t pid, int* status, int options, struct __kernel_rusage* if (status && test_user_memory(status, sizeof(*status), /*write=*/true)) return -EFAULT; - /* Prepare options for shim_do_waitid(). */ + /* Prepare options for do_waitid(). */ options |= WEXITED; if (options & WUNTRACED) { options &= ~WUNTRACED; @@ -259,7 +265,7 @@ long shim_do_wait4(pid_t pid, int* status, int options, struct __kernel_rusage* } info.si_pid = 0; - int ret = shim_do_waitid(which, id, &info, options, ru); + int ret = do_waitid(which, id, &info, options); if (ret < 0) return ret; diff --git a/LibOS/shim/test/ltp/ltp.cfg b/LibOS/shim/test/ltp/ltp.cfg index d99dc04f..f3c482b1 100644 --- a/LibOS/shim/test/ltp/ltp.cfg +++ b/LibOS/shim/test/ltp/ltp.cfg @@ -2055,8 +2055,6 @@ skip = yes skip = yes # EINVAL from native sendmsg() -# also, shim_sendmmsg() has wrong vlen handling? -# issue: https://github.com/oscarlab/graphene/issues/1931 [sendmmsg01] skip = yes