[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 02/16] linux-user: fix netlink memory corruption
From: |
riku . voipio |
Subject: |
[Qemu-devel] [PULL 02/16] linux-user: fix netlink memory corruption |
Date: |
Tue, 19 Jul 2016 15:54:00 +0300 |
From: Laurent Vivier <address@hidden>
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)
<here req.nlh.nlmsg_seq has been byte-swapped>
...
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))
{
<we compare nlmh->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 <address@hidden>
Signed-off-by: Riku Voipio <address@hidden>
---
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 59defff..491ab55 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;
}
--
2.1.4
- [Qemu-devel] [PULL 00/16] linux-user before 2.7 hardfreeze, riku . voipio, 2016/07/19
- [Qemu-devel] [PULL 03/16] linux-user: add fd_trans helper in do_recvfrom(), riku . voipio, 2016/07/19
- [Qemu-devel] [PULL 01/16] linux-user: fd_trans_*_data() returns the length, riku . voipio, 2016/07/19
- [Qemu-devel] [PULL 04/16] linux-user: convert sockaddr_ll from host to target, riku . voipio, 2016/07/19
- [Qemu-devel] [PULL 02/16] linux-user: fix netlink memory corruption,
riku . voipio <=
- [Qemu-devel] [PULL 06/16] linux-user: Check sigsetsize argument to syscalls, riku . voipio, 2016/07/19
- [Qemu-devel] [PULL 07/16] linux-user: Add loop control ioctls, riku . voipio, 2016/07/19
- [Qemu-devel] [PULL 08/16] linux-user: Correct type for BLKSSZGET, riku . voipio, 2016/07/19
- [Qemu-devel] [PULL 10/16] linux-user: Forget about synchronous signal once it is delivered, riku . voipio, 2016/07/19
- [Qemu-devel] [PULL 05/16] linux-user: add nested netlink types, riku . voipio, 2016/07/19
- [Qemu-devel] [PULL 11/16] linux-user: Handle short lengths in host_to_target_sockaddr(), riku . voipio, 2016/07/19
- [Qemu-devel] [PULL 09/16] linux-user: Correct type for LOOP_GET_STATUS{, 64} ioctls, riku . voipio, 2016/07/19
- [Qemu-devel] [PULL 12/16] linux-user: Add some new blk ioctls, riku . voipio, 2016/07/19
- [Qemu-devel] [PULL 13/16] TIOCGPTN and related terminal control ioctls were not converted to the guest ioctl format on x86_64 targets. Convert these ioctls to enable terminal functionality on x86_64 guests., riku . voipio, 2016/07/19
- [Qemu-devel] [PULL 15/16] linux-user: Fix type for SIOCATMARK ioctl, riku . voipio, 2016/07/19