From cf89a0b7d0961f27a75bfbcf075d9e87764c568b Mon Sep 17 00:00:00 2001 From: Pavel Tikhomirov Date: Mon, 6 Jun 2022 16:13:56 +0300 Subject: [PATCH] sk-queue: fix dumping queue packets When we have several SCM_RIGHTS packets and at least one SCM_CREDENTIALS sent simultaneousely into same unix socket we see that SCM_RIGHTS are dumped wrong. It happens because before this patch SkPacketEntry had been reused when dumping consequent packets, thus when dumping SCM_RIGHTS for non-first packet it's pe.scm also contained SCM_RIGHTS for previous packet. To fix this let's not reuse SkPacketEntry. Also while on it let's split dump_sk_queue_packet() - separate function for dumping one packet, to add more readability to the code. Also drop meaningless "ret = -E2BIG" and staff. Note that after this change pe.ucred is also not reused as it probably should've always been. Fixes: e4956b0b1 ("sk-queue: Add support of ucreds in unix sockets") Signed-off-by: Pavel Tikhomirov (cherry picked from commit 6832ccd499c1bef9bc220cdf42202c468fe5a37c) Signed-off-by: Ahmed Elaidy Reviewed-by: Alexander Mikhalitsyn --- criu/sk-queue.c | 164 ++++++++++++++++++++++++------------------------ 1 file changed, 81 insertions(+), 83 deletions(-) diff --git a/criu/sk-queue.c b/criu/sk-queue.c index 191c9a034..46ef5bcb8 100644 --- a/criu/sk-queue.c +++ b/criu/sk-queue.c @@ -398,10 +398,81 @@ static int dump_packet_cmsg(struct msghdr *mh, SkPacketEntry *pe, int flags) return ret; } -int dump_sk_queue(int sock_fd, int sock_id, int flags) +static int dump_sk_queue_packet(int sock_fd, int sock_id, void *data, int size, int flags) { SkPacketEntry pe = SK_PACKET_ENTRY__INIT; + int ret, exit_code = -1; + char cmsg[CMSG_MAX_SIZE]; + struct iovec iov = { + .iov_base = data, + .iov_len = size, + }; + struct msghdr msg = { + .msg_iov = &iov, + .msg_iovlen = 1, + .msg_control = &cmsg, + .msg_controllen = sizeof(cmsg), + }; + + pe.id_for = sock_id; + + ret = pe.length = recvmsg(sock_fd, &msg, MSG_DONTWAIT | MSG_PEEK); + if (!ret) { + /* + * It means, that peer has performed an + * orderly shutdown, so we're done. + */ + return 1; + } else if (ret < 0) { + if (errno == EAGAIN) + return 1; + + pr_perror("recvmsg fail: error"); + return -1; + } + + if (msg.msg_flags & MSG_TRUNC) { + /* + * DGRAM truncated. This should not happen. But we have + * to check... + */ + pr_err("recvmsg failed: truncated\n"); + return -1; + } + + ret = dump_packet_cmsg(&msg, &pe, flags); + if (ret < 0) + goto cleanup_packet; + + if (ret > 0) { + if (ret == 1) { + if (queue_packet_entry(&pe, data, pe.length)) + goto cleanup_packet; + } + exit_code = 0; + goto cleanup_packet; + } + + ret = pb_write_one(img_from_set(glob_imgset, CR_FD_SK_QUEUES), &pe, PB_SK_QUEUES); + if (ret < 0) + goto cleanup_packet; + + ret = write_img_buf(img_from_set(glob_imgset, CR_FD_SK_QUEUES), data, pe.length); + if (ret < 0) + goto cleanup_packet; + + exit_code = 0; +cleanup_packet: + if (pe.scm) + release_cmsg(&pe); + xfree(pe.ucred); + return exit_code; +} + +int dump_sk_queue(int sock_fd, int sock_id, int flags) +{ int ret, size, orig_peek_off; + int exit_code = -1; void *data; socklen_t tmp; @@ -413,7 +484,7 @@ int dump_sk_queue(int sock_fd, int sock_id, int flags) ret = getsockopt(sock_fd, SOL_SOCKET, SO_PEEK_OFF, &orig_peek_off, &tmp); if (ret < 0) { pr_perror("getsockopt failed"); - return ret; + return -1; } /* * Discover max DGRAM size @@ -423,7 +494,7 @@ int dump_sk_queue(int sock_fd, int sock_id, int flags) ret = getsockopt(sock_fd, SOL_SOCKET, SO_SNDBUF, &size, &tmp); if (ret < 0) { pr_perror("getsockopt failed"); - return ret; + return -1; } /* Note: 32 bytes will be used by kernel for protocol header. */ @@ -442,87 +513,17 @@ int dump_sk_queue(int sock_fd, int sock_id, int flags) ret = setsockopt(sock_fd, SOL_SOCKET, SO_PEEK_OFF, &ret, sizeof(int)); if (ret < 0) { pr_perror("setsockopt fail"); - goto err_brk; + goto err_free; } - pe.id_for = sock_id; - while (1) { - char cmsg[CMSG_MAX_SIZE]; - struct iovec iov = { - .iov_base = data, - .iov_len = size, - }; - struct msghdr msg = { - .msg_iov = &iov, - .msg_iovlen = 1, - .msg_control = &cmsg, - .msg_controllen = sizeof(cmsg), - }; - - ret = pe.length = recvmsg(sock_fd, &msg, MSG_DONTWAIT | MSG_PEEK); - if (!ret) - /* - * It means, that peer has performed an - * orderly shutdown, so we're done. - */ + ret = dump_sk_queue_packet(sock_fd, sock_id, data, size, flags); + if (ret == 1) break; - else if (ret < 0) { - if (errno == EAGAIN) - break; /* we're done */ - pr_perror("recvmsg fail: error"); + else if (ret == -1) goto err_set_sock; - } - if (msg.msg_flags & MSG_TRUNC) { - /* - * DGRAM truncated. This should not happen. But we have - * to check... - */ - pr_err("sys_recvmsg failed: truncated\n"); - ret = -E2BIG; - goto err_set_sock; - } - if (msg.msg_flags & MSG_CTRUNC) { - /* - * Control data truncated. This means the cmsg - * buffer was too small and SCM data (such as - * passed file descriptors) has been silently - * discarded by the kernel. - */ - pr_err("sys_recvmsg failed: control data truncated\n"); - ret = -E2BIG; - goto err_set_sock; - } - - ret = dump_packet_cmsg(&msg, &pe, flags); - if (ret < 0) - goto err_set_sock; - - if (ret > 0) { - if (ret == 1) { - ret = -1; - if (queue_packet_entry(&pe, data, pe.length)) - goto err_set_sock; - } - continue; - } - - ret = pb_write_one(img_from_set(glob_imgset, CR_FD_SK_QUEUES), &pe, PB_SK_QUEUES); - if (ret < 0) { - ret = -EIO; - goto err_set_sock; - } - - ret = write_img_buf(img_from_set(glob_imgset, CR_FD_SK_QUEUES), data, pe.length); - if (ret < 0) { - ret = -EIO; - goto err_set_sock; - } - - if (pe.scm) - release_cmsg(&pe); } - ret = 0; + exit_code = 0; err_set_sock: /* @@ -532,12 +533,9 @@ err_set_sock: pr_perror("setsockopt failed on restore"); ret = -1; } - if (pe.scm) - release_cmsg(&pe); -err_brk: - xfree(pe.ucred); +err_free: xfree(data); - return ret; + return exit_code; } static int send_one_pkt(int fd, struct sk_packet *pkt)