vhost: avoid enum fields in VhostUserMsg
authorStefan Hajnoczi <stefanha@redhat.com>
Mon, 5 Feb 2018 12:16:00 +0000 (13:16 +0100)
committerFerruh Yigit <ferruh.yigit@intel.com>
Fri, 30 Mar 2018 12:08:42 +0000 (14:08 +0200)
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 <stefanha@redhat.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
lib/librte_vhost/vhost_user.h

index d4bd604..0fafbe6 100644 (file)
@@ -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