Skip to content

Commit

Permalink
linux-user: Fix length handling in host_to_target_cmsg
Browse files Browse the repository at this point in the history
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 <peter.maydell@linaro.org>
Signed-off-by: Riku Voipio <riku.voipio@linaro.org>
  • Loading branch information
pm215 authored and Riku Voipio committed Jun 16, 2015
1 parent 79cb1f1 commit c2aeb25
Showing 1 changed file with 61 additions and 8 deletions.
69 changes: 61 additions & 8 deletions linux-user/syscall.c
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand All @@ -1281,16 +1295,43 @@ 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) {
case SCM_RIGHTS:
{
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]);
Expand All @@ -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);
Expand All @@ -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);
}
Expand Down

0 comments on commit c2aeb25

Please sign in to comment.