From patchwork Mon Jun 15 12:20:48 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Riku Voipio X-Patchwork-Id: 484281 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 265B51401F6 for ; Mon, 15 Jun 2015 22:33:38 +1000 (AEST) Received: from localhost ([::1]:33976 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4Ta4-0007X1-8o for incoming@patchwork.ozlabs.org; Mon, 15 Jun 2015 08:33:36 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49765) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4TVn-0008Cn-QB for qemu-devel@nongnu.org; Mon, 15 Jun 2015 08:29:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z4TVj-0006Tz-O4 for qemu-devel@nongnu.org; Mon, 15 Jun 2015 08:29:11 -0400 Received: from mail-la0-f41.google.com ([209.85.215.41]:34742) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z4TVj-0006Tp-Cc for qemu-devel@nongnu.org; Mon, 15 Jun 2015 08:29:07 -0400 Received: by labbc20 with SMTP id bc20so20478901lab.1 for ; Mon, 15 Jun 2015 05:29:06 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=njXWL6EOwPYveUWkvo2/MeEsEX0RbmdwxickgsAFmew=; b=IGqKDvHuV0nmwfYeBLyK0bkBcx9znOdmB36W/2QabW0psFykPW6mw3Gzklpn6gY4Gj pxsD4YQBEYGiCGSAilZKyUgyilGzDqGoPvKnYuyB0qL5mhJZr09Two2l6cqd0JjvDvtX nbveaiFsib8aFkiX0fW0ayJ9aN+Vawd7DKKWkjjZBG6sIN9+rxlaMGwCFH14wQ94vSol WXKKi1VX/I9/mpwTiPmvi4iucY8bLRxyjKibtE6ZN9sCATU6ioGIKqtx4fPVoDCg0dvh IL5w251aFwVgXUoPH6ShNx27VTXhQbN6CNJQSFK8B4EFL1C8QXnRurjCFCVE8KySPviF bGYQ== X-Gm-Message-State: ALoCoQlJf2n3HZWaEylVKQyLxpH6SFGFwYjP1rNmiJCozkr67rUVGF2C2wSYh6q4JQD1R0XcfG78 X-Received: by 10.152.206.10 with SMTP id lk10mr26777026lac.90.1434370856376; Mon, 15 Jun 2015 05:20:56 -0700 (PDT) Received: from localhost.localdomain (91-157-196-38.elisa-laajakaista.fi. [91.157.196.38]) by mx.google.com with ESMTPSA id bm5sm2683890lbc.45.2015.06.15.05.20.55 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 15 Jun 2015 05:20:55 -0700 (PDT) From: riku.voipio@linaro.org To: qemu-devel@nongnu.org Date: Mon, 15 Jun 2015 15:20:48 +0300 Message-Id: X-Mailer: git-send-email 2.1.4 In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.85.215.41 Cc: peter.maydell@linaro.org Subject: [Qemu-devel] [PULL 4/6] linux-user: Fix length handling in host_to_target_cmsg X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: Peter Maydell 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(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index ed8c423..839ce93 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); }