From patchwork Mon Feb 5 12:16:36 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Hajnoczi X-Patchwork-Id: 34963 X-Patchwork-Delegate: maxime.coquelin@redhat.com Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 13ED81B34B; Mon, 5 Feb 2018 13:17:06 +0100 (CET) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 9EF961B33B for ; Mon, 5 Feb 2018 13:17:04 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1242AC036743; Mon, 5 Feb 2018 12:17:04 +0000 (UTC) Received: from localhost (ovpn-117-200.ams2.redhat.com [10.36.117.200]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1F7E65D6A8; Mon, 5 Feb 2018 12:16:58 +0000 (UTC) From: Stefan Hajnoczi To: dev@dpdk.org Cc: Maxime Coquelin , Yuanhan Liu , Stefan Hajnoczi Date: Mon, 5 Feb 2018 12:16:36 +0000 Message-Id: <20180205121642.26428-3-stefanha@redhat.com> In-Reply-To: <20180205121642.26428-1-stefanha@redhat.com> References: <20180205121642.26428-1-stefanha@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Mon, 05 Feb 2018 12:17:04 +0000 (UTC) Subject: [dpdk-dev] [PATCH 2/8] vhost: avoid enum fields in VhostUserMsg X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" The VhostUserMsg struct binary representation must match the vhost-user protocol specification since this struct is read from and written to the socket. The VhostUserMsg.request union contains enum fields. Enum binary representation is implementation-defined according to the C standard and it is unportable to make assumptions about the representation: 6.7.2.2 Enumeration specifiers ... Each enumerated type shall be compatible with char, a signed integer type, or an unsigned integer type. The choice of type is implementation-defined, but shall be capable of representing the values of all the members of the enumeration. Additionally, librte_vhost relies on the enum type being unsigned when validating untrusted inputs: if (ret <= 0 || msg.request.master >= VHOST_USER_MAX) { If msg.request.master is signed then negative values pass this check! Even if we assume gcc on x86_64 (SysV amd64 ABI) and don't care about portability, the actual enum constants still affect the final type. For example, if we add a negative constant then its type changes to signed int: typedef enum VhostUserRequest { ... VHOST_USER_INVALID = -1, }; This is very fragile and it's unlikely that anyone changing the code would remember this. A security hole can be introduced accidentally. This patch switches VhostUserMsg.request fields to uint32_t to avoid the portability and potential security issues. Signed-off-by: Stefan Hajnoczi --- lib/librte_vhost/vhost_user.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/librte_vhost/vhost_user.h b/lib/librte_vhost/vhost_user.h index d4bd604b9..0fafbe6e0 100644 --- a/lib/librte_vhost/vhost_user.h +++ b/lib/librte_vhost/vhost_user.h @@ -81,8 +81,8 @@ typedef struct VhostUserLog { typedef struct VhostUserMsg { union { - VhostUserRequest master; - VhostUserSlaveRequest slave; + uint32_t master; /* a VhostUserRequest value */ + uint32_t slave; /* a VhostUserSlaveRequest value*/ } request; #define VHOST_USER_VERSION_MASK 0x3