From 8be656b87c6bb1b9f8af3ff78094413d71e4443a Mon Sep 17 00:00:00 2001 From: Alexander Graf Date: Wed, 6 May 2015 23:47:32 +0200 Subject: linux-user: Allocate thunk size dynamically We store all struct types in an array of static size without ever checking whether we overrun it. Of course some day someone (like me in another, ancient ALSA enabling patch set) will run into the limit without realizing it. So let's make the allocation dynamic. We already know the number of structs that we want to allocate, so we only need to pass the variable into the respective piece of code. Also, to ensure we don't accidently overwrite random memory, add some asserts to sanity check whether a thunk is actually part of our array. Signed-off-by: Alexander Graf Signed-off-by: Riku Voipio --- linux-user/syscall.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'linux-user/syscall.c') diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 1622ad6490..f56f3e0431 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -3277,6 +3277,7 @@ static abi_long do_ipc(unsigned int call, abi_long first, #define STRUCT_SPECIAL(name) STRUCT_ ## name, enum { #include "syscall_types.h" +STRUCT_MAX }; #undef STRUCT #undef STRUCT_SPECIAL @@ -4879,6 +4880,8 @@ void syscall_init(void) int size; int i; + thunk_init(STRUCT_MAX); + #define STRUCT(name, ...) thunk_register_struct(STRUCT_ ## name, #name, struct_ ## name ## _def); #define STRUCT_SPECIAL(name) thunk_register_struct_direct(STRUCT_ ## name, #name, &struct_ ## name ## _def); #include "syscall_types.h" -- cgit v1.2.3-55-g7522 From c2aeb2586bd258ad76fcfe308f883075e73ff1d2 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 26 May 2015 19:46:31 +0100 Subject: linux-user: Fix length handling in host_to_target_cmsg The previous code for handling payload length when converting cmsg structures from host to target had a number of problems: * we required the msg->msg_controllen to declare the buffer to have enough space for final trailing padding (we were checking against CMSG_SPACE), whereas the kernel does not require this, and common userspace code assumes this. (In particular, glibc's "try to talk to nscd" code that it will run on startup will receive a cmsg with a 4 byte payload and only allocate 4 bytes for it, which was causing us to do the wrong thing on architectures that need 8-alignment.) * we weren't correctly handling the fact that the SO_TIMESTAMP payload may be larger for the target than the host * we weren't marking the messages with MSG_CTRUNC when we did need to truncate a message that wasn't truncated by the host, but were instead logging a QEMU message; since truncation is always the result of a guest giving us an insufficiently sized buffer, we should report it to the guest as the kernel does and don't log anything Rewrite the parts of the function that deal with length to fix these issues, and add a comment in target_to_host_cmsg to explain why the overflow logging it does is a QEMU bug, not a guest issue. Signed-off-by: Peter Maydell Signed-off-by: Riku Voipio --- linux-user/syscall.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 61 insertions(+), 8 deletions(-) (limited to 'linux-user/syscall.c') diff --git a/linux-user/syscall.c b/linux-user/syscall.c index f56f3e0431..15b1e81471 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -1202,6 +1202,15 @@ static inline abi_long target_to_host_cmsg(struct msghdr *msgh, space += CMSG_SPACE(len); if (space > msgh->msg_controllen) { space -= CMSG_SPACE(len); + /* This is a QEMU bug, since we allocated the payload + * area ourselves (unlike overflow in host-to-target + * conversion, which is just the guest giving us a buffer + * that's too small). It can't happen for the payload types + * we currently support; if it becomes an issue in future + * we would need to improve our allocation strategy to + * something more intelligent than "twice the size of the + * target buffer we're reading from". + */ gemu_log("Host cmsg overflow\n"); break; } @@ -1267,11 +1276,16 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh, void *target_data = TARGET_CMSG_DATA(target_cmsg); int len = cmsg->cmsg_len - CMSG_ALIGN(sizeof (struct cmsghdr)); + int tgt_len, tgt_space; - space += TARGET_CMSG_SPACE(len); - if (space > msg_controllen) { - space -= TARGET_CMSG_SPACE(len); - gemu_log("Target cmsg overflow\n"); + /* We never copy a half-header but may copy half-data; + * this is Linux's behaviour in put_cmsg(). Note that + * truncation here is a guest problem (which we report + * to the guest via the CTRUNC bit), unlike truncation + * in target_to_host_cmsg, which is a QEMU bug. + */ + if (msg_controllen < sizeof(struct cmsghdr)) { + target_msgh->msg_flags |= tswap32(MSG_CTRUNC); break; } @@ -1281,8 +1295,35 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh, target_cmsg->cmsg_level = tswap32(cmsg->cmsg_level); } target_cmsg->cmsg_type = tswap32(cmsg->cmsg_type); - target_cmsg->cmsg_len = tswapal(TARGET_CMSG_LEN(len)); + tgt_len = TARGET_CMSG_LEN(len); + + /* Payload types which need a different size of payload on + * the target must adjust tgt_len here. + */ + switch (cmsg->cmsg_level) { + case SOL_SOCKET: + switch (cmsg->cmsg_type) { + case SO_TIMESTAMP: + tgt_len = sizeof(struct target_timeval); + break; + default: + break; + } + default: + break; + } + + if (msg_controllen < tgt_len) { + target_msgh->msg_flags |= tswap32(MSG_CTRUNC); + tgt_len = msg_controllen; + } + + /* We must now copy-and-convert len bytes of payload + * into tgt_len bytes of destination space. Bear in mind + * that in both source and destination we may be dealing + * with a truncated value! + */ switch (cmsg->cmsg_level) { case SOL_SOCKET: switch (cmsg->cmsg_type) { @@ -1290,7 +1331,7 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh, { int *fd = (int *)data; int *target_fd = (int *)target_data; - int i, numfds = len / sizeof(int); + int i, numfds = tgt_len / sizeof(int); for (i = 0; i < numfds; i++) target_fd[i] = tswap32(fd[i]); @@ -1302,8 +1343,10 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh, struct target_timeval *target_tv = (struct target_timeval *)target_data; - if (len != sizeof(struct timeval)) + if (len != sizeof(struct timeval) || + tgt_len != sizeof(struct target_timeval)) { goto unimplemented; + } /* copy struct timeval to target */ target_tv->tv_sec = tswapal(tv->tv_sec); @@ -1330,9 +1373,19 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh, unimplemented: gemu_log("Unsupported ancillary data: %d/%d\n", cmsg->cmsg_level, cmsg->cmsg_type); - memcpy(target_data, data, len); + memcpy(target_data, data, MIN(len, tgt_len)); + if (tgt_len > len) { + memset(target_data + len, 0, tgt_len - len); + } } + target_cmsg->cmsg_len = tswapal(tgt_len); + tgt_space = TARGET_CMSG_SPACE(tgt_len); + if (msg_controllen < tgt_space) { + tgt_space = msg_controllen; + } + msg_controllen -= tgt_space; + space += tgt_space; cmsg = CMSG_NXTHDR(msgh, cmsg); target_cmsg = TARGET_CMSG_NXTHDR(target_msgh, target_cmsg); } -- cgit v1.2.3-55-g7522 From 876e23cb2e545148a0ee4effda5c63c861855941 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 26 May 2015 19:46:32 +0100 Subject: linux-user: use __get_user and __put_user in cmsg conversions The target payloads in cmsg conversions may not have the alignment required by the host. Using the get_user and put_user functions is the easiest way to handle this and also do the byte-swapping we require. (Note that prior to this commit target_to_host_cmsg was incorrectly using __put_user() rather than __get_user() for the SCM_CREDENTIALS conversion, which meant it wasn't getting the benefit of the misalignment handling.) Signed-off-by: Peter Maydell Signed-off-by: Riku Voipio --- linux-user/syscall.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) (limited to 'linux-user/syscall.c') diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 15b1e81471..5cf265cb08 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -1228,17 +1228,18 @@ static inline abi_long target_to_host_cmsg(struct msghdr *msgh, int *target_fd = (int *)target_data; int i, numfds = len / sizeof(int); - for (i = 0; i < numfds; i++) - fd[i] = tswap32(target_fd[i]); + for (i = 0; i < numfds; i++) { + __get_user(fd[i], target_fd + i); + } } else if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_CREDENTIALS) { struct ucred *cred = (struct ucred *)data; struct target_ucred *target_cred = (struct target_ucred *)target_data; - __put_user(target_cred->pid, &cred->pid); - __put_user(target_cred->uid, &cred->uid); - __put_user(target_cred->gid, &cred->gid); + __get_user(cred->pid, &target_cred->pid); + __get_user(cred->uid, &target_cred->uid); + __get_user(cred->gid, &target_cred->gid); } else { gemu_log("Unsupported ancillary data: %d/%d\n", cmsg->cmsg_level, cmsg->cmsg_type); @@ -1333,8 +1334,9 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh, int *target_fd = (int *)target_data; int i, numfds = tgt_len / sizeof(int); - for (i = 0; i < numfds; i++) - target_fd[i] = tswap32(fd[i]); + for (i = 0; i < numfds; i++) { + __put_user(fd[i], target_fd + i); + } break; } case SO_TIMESTAMP: @@ -1349,8 +1351,8 @@ static inline abi_long host_to_target_cmsg(struct target_msghdr *target_msgh, } /* copy struct timeval to target */ - target_tv->tv_sec = tswapal(tv->tv_sec); - target_tv->tv_usec = tswapal(tv->tv_usec); + __put_user(tv->tv_sec, &target_tv->tv_sec); + __put_user(tv->tv_usec, &target_tv->tv_usec); break; } case SCM_CREDENTIALS: -- cgit v1.2.3-55-g7522 From 45c874ebbae661238bfa3c0534480ebe2940b112 Mon Sep 17 00:00:00 2001 From: Laurent Vivier Date: Tue, 16 Jun 2015 00:35:28 +0200 Subject: linux-user: ioctl() command type is int When executing a 64bit target chroot on 64bit host, the ioctl() command can mismatch. It seems the previous commit doesn't solve the problem in my case: 9c6bf9c7 linux-user: Fix ioctl cmd type mismatch on 64-bit targets For example, a ppc64 chroot on an x86_64 host: bash-4.3# ls Unsupported ioctl: cmd=0x80087467 Unsupported ioctl: cmd=0x802c7415 The origin of the problem is in syscall.c:do_ioctl(). static abi_long do_ioctl(int fd, abi_long cmd, abi_long arg) In this case (ppc64) abi_long is long (on the x86_64), and cmd = 0x0000000080087467 then if (ie->target_cmd == cmd) target_cmd is int, so target_cmd = 0x80087467 and to compare an int with a long, the sign is extended to 64bit, so the comparison is: if (0xffffffff80087467 == 0x0000000080087467) which doesn't match whereas it should. This patch uses int in the case of the target command type instead of abi_long. Signed-off-by: Laurent Vivier Signed-off-by: Riku Voipio --- linux-user/syscall.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'linux-user/syscall.c') diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 5cf265cb08..f62c698948 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -3346,7 +3346,7 @@ STRUCT_MAX typedef struct IOCTLEntry IOCTLEntry; typedef abi_long do_ioctl_fn(const IOCTLEntry *ie, uint8_t *buf_temp, - int fd, abi_long cmd, abi_long arg); + int fd, int cmd, abi_long arg); struct IOCTLEntry { int target_cmd; @@ -3372,7 +3372,7 @@ struct IOCTLEntry { / sizeof(struct fiemap_extent)) static abi_long do_ioctl_fs_ioc_fiemap(const IOCTLEntry *ie, uint8_t *buf_temp, - int fd, abi_long cmd, abi_long arg) + int fd, int cmd, abi_long arg) { /* The parameter for this ioctl is a struct fiemap followed * by an array of struct fiemap_extent whose size is set @@ -3453,7 +3453,7 @@ static abi_long do_ioctl_fs_ioc_fiemap(const IOCTLEntry *ie, uint8_t *buf_temp, #endif static abi_long do_ioctl_ifconf(const IOCTLEntry *ie, uint8_t *buf_temp, - int fd, abi_long cmd, abi_long arg) + int fd, int cmd, abi_long arg) { const argtype *arg_type = ie->arg_type; int target_size; @@ -3547,7 +3547,7 @@ static abi_long do_ioctl_ifconf(const IOCTLEntry *ie, uint8_t *buf_temp, } static abi_long do_ioctl_dm(const IOCTLEntry *ie, uint8_t *buf_temp, int fd, - abi_long cmd, abi_long arg) + int cmd, abi_long arg) { void *argptr; struct dm_ioctl *host_dm; @@ -3772,7 +3772,7 @@ out: } static abi_long do_ioctl_blkpg(const IOCTLEntry *ie, uint8_t *buf_temp, int fd, - abi_long cmd, abi_long arg) + int cmd, abi_long arg) { void *argptr; int target_size; @@ -3825,7 +3825,7 @@ out: } static abi_long do_ioctl_rt(const IOCTLEntry *ie, uint8_t *buf_temp, - int fd, abi_long cmd, abi_long arg) + int fd, int cmd, abi_long arg) { const argtype *arg_type = ie->arg_type; const StructEntry *se; @@ -3888,7 +3888,7 @@ static abi_long do_ioctl_rt(const IOCTLEntry *ie, uint8_t *buf_temp, } static abi_long do_ioctl_kdsigaccept(const IOCTLEntry *ie, uint8_t *buf_temp, - int fd, abi_long cmd, abi_long arg) + int fd, int cmd, abi_long arg) { int sig = target_to_host_signal(arg); return get_errno(ioctl(fd, ie->host_cmd, sig)); @@ -3905,7 +3905,7 @@ static IOCTLEntry ioctl_entries[] = { /* ??? Implement proper locking for ioctls. */ /* do_ioctl() Must return target values and target errnos. */ -static abi_long do_ioctl(int fd, abi_long cmd, abi_long arg) +static abi_long do_ioctl(int fd, int cmd, abi_long arg) { const IOCTLEntry *ie; const argtype *arg_type; -- cgit v1.2.3-55-g7522