From 7d61d892327d803ae43d14500601e48031b4632c Mon Sep 17 00:00:00 2001 From: Laurent Vivier Date: Tue, 21 Jun 2016 19:51:14 +0200 Subject: [PATCH] linux-user: fix netlink memory corruption Netlink is byte-swapping data in the guest memory (it's bad). It's ok when the data come from the host as they are generated by the host. But it doesn't work when data come from the guest: the guest can try to reuse these data whereas they have been byte-swapped. This is what happens in glibc: glibc generates a sequence number in nlh.nlmsg_seq and calls sendto() with this nlh. In sendto(), we byte-swap nlmsg.seq. Later, after the recvmsg(), glibc compares nlh.nlmsg_seq with sequence number given in return, and of course it fails (hangs), because nlh.nlmsg_seq is not valid anymore. The involved code in glibc is: sysdeps/unix/sysv/linux/check_pf.c:make_request() ... req.nlh.nlmsg_seq = time (NULL); ... if (TEMP_FAILURE_RETRY (__sendto (fd, (void *) &req, sizeof (req), 0, (struct sockaddr *) &nladdr, sizeof (nladdr))) < 0) ... do { ... ssize_t read_len = TEMP_FAILURE_RETRY (__recvmsg (fd, &msg, 0)); ... struct nlmsghdr *nlmh; for (nlmh = (struct nlmsghdr *) buf; NLMSG_OK (nlmh, (size_t) read_len); nlmh = (struct nlmsghdr *) NLMSG_NEXT (nlmh, read_len)) { nlmsg_seq with corrupted req.nlh.nlmsg_seq> if (nladdr.nl_pid != 0 || (pid_t) nlmh->nlmsg_pid != pid || nlmh->nlmsg_seq != req.nlh.nlmsg_seq) continue; ... else if (nlmh->nlmsg_type == NLMSG_DONE) /* We found the end, leave the loop. */ done = true; } } while (! done); As we have a continue on "nlmh->nlmsg_seq != req.nlh.nlmsg_seq", "done" cannot be set to "true" and we have an infinite loop. It's why commands like "apt-get update" or "dnf update hangs". Signed-off-by: Laurent Vivier Signed-off-by: Riku Voipio --- linux-user/syscall.c | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 59defff01f..491ab55d62 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -3017,13 +3017,22 @@ static abi_long do_sendrecvmsg_locked(int fd, struct target_msghdr *msgp, if (send) { if (fd_trans_target_to_host_data(fd)) { - ret = fd_trans_target_to_host_data(fd)(msg.msg_iov->iov_base, + void *host_msg; + + host_msg = g_malloc(msg.msg_iov->iov_len); + memcpy(host_msg, msg.msg_iov->iov_base, msg.msg_iov->iov_len); + ret = fd_trans_target_to_host_data(fd)(host_msg, msg.msg_iov->iov_len); + if (ret >= 0) { + msg.msg_iov->iov_base = host_msg; + ret = get_errno(safe_sendmsg(fd, &msg, flags)); + } + g_free(host_msg); } else { ret = target_to_host_cmsg(&msg, msgp); - } - if (ret == 0) { - ret = get_errno(safe_sendmsg(fd, &msg, flags)); + if (ret == 0) { + ret = get_errno(safe_sendmsg(fd, &msg, flags)); + } } } else { ret = get_errno(safe_recvmsg(fd, &msg, flags)); @@ -3239,6 +3248,7 @@ static abi_long do_sendto(int fd, abi_ulong msg, size_t len, int flags, { void *addr; void *host_msg; + void *copy_msg = NULL; abi_long ret; if ((int)addrlen < 0) { @@ -3249,23 +3259,29 @@ static abi_long do_sendto(int fd, abi_ulong msg, size_t len, int flags, if (!host_msg) return -TARGET_EFAULT; if (fd_trans_target_to_host_data(fd)) { + copy_msg = host_msg; + host_msg = g_malloc(len); + memcpy(host_msg, copy_msg, len); ret = fd_trans_target_to_host_data(fd)(host_msg, len); if (ret < 0) { - unlock_user(host_msg, msg, 0); - return ret; + goto fail; } } if (target_addr) { addr = alloca(addrlen+1); ret = target_to_host_sockaddr(fd, addr, target_addr, addrlen); if (ret) { - unlock_user(host_msg, msg, 0); - return ret; + goto fail; } ret = get_errno(safe_sendto(fd, host_msg, len, flags, addr, addrlen)); } else { ret = get_errno(safe_sendto(fd, host_msg, len, flags, NULL, 0)); } +fail: + if (copy_msg) { + g_free(host_msg); + host_msg = copy_msg; + } unlock_user(host_msg, msg, 0); return ret; }