>
> This allows running big endian and little endian guest side by side using
> cut & paste between them.
> 
> There is a general design idea that swapping should come as close to
> virtio_read/virtio_write as possible. In particular, the protocol
> between vdagent and vdagentd is guest-specific and in native endian.
> With muliple layers of headers this is a bit tricky. A few message types
> have to be swapped fully before passing through vdagentd.
> 
> Signed-off-by: Michal Suchanek <msucha...@suse.de>
> 
> ---
> v2:
>  - introduce helper functions to swap (a portion of) a message wholesale
>  - pollute fewer places with swapping sometimes at the cost of slightly
>    more verbose code
> v3:
>  - use glib byteswap macros in place of endian.h byteswap macros
>  - move variable declaration out of case statement
>  - reuse more of existing clipboard code
> ---
>  src/vdagentd/vdagentd.c    | 89
>  +++++++++++++++++++++++++++++++++++++++-------
>  src/vdagentd/virtio-port.c | 36 ++++++++++++-------
>  2 files changed, 100 insertions(+), 25 deletions(-)
> 
> diff --git a/src/vdagentd/vdagentd.c b/src/vdagentd/vdagentd.c
> index a1faf23..4cac473 100644
> --- a/src/vdagentd/vdagentd.c
> +++ b/src/vdagentd/vdagentd.c
> @@ -78,6 +78,34 @@ static int client_connected = 0;
>  static int max_clipboard = -1;
>  
>  /* utility functions */
> +static void virtio_msg_uint32_to_le(uint8_t *_msg, uint32_t size, uint32_t
> offset)
> +{
> +    uint32_t i, *msg = (uint32_t *)(_msg + offset);
> +
> +    /* offset - size % 4 should be 0 */
> +    for (i = 0; i < (size - offset) / 4; i++)
> +        msg[i] = GUINT32_TO_LE(msg[i]);
> +}
> +
> +static void virtio_msg_uint32_from_le(uint8_t *_msg, uint32_t size, uint32_t
> offset)
> +{
> +    uint32_t i, *msg = (uint32_t *)(_msg + offset);
> +
> +    /* offset - size % 4 should be 0 */
> +    for (i = 0; i < (size - offset) / 4; i++)
> +        msg[i] = GUINT32_FROM_LE(msg[i]);
> +}
> +
> +static void virtio_msg_uint16_from_le(uint8_t *_msg, uint32_t size, uint32_t
> offset)
> +{
> +    uint32_t i;
> +    uint16_t *msg = (uint16_t *)(_msg + offset);
> +
> +    /* offset - size % 2 should be 0 */
> +    for (i = 0; i < (size - offset) / 2; i++)
> +        msg[i] = GUINT16_FROM_LE(msg[i]);
> +}
> +
>  /* vdagentd <-> spice-client communication handling */
>  static void send_capabilities(struct vdagent_virtio_port *vport,
>      uint32_t request)
> @@ -102,6 +130,7 @@ static void send_capabilities(struct vdagent_virtio_port
> *vport,
>      VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_GUEST_LINEEND_LF);
>      VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_MAX_CLIPBOARD);
>      VD_AGENT_SET_CAPABILITY(caps->caps, VD_AGENT_CAP_AUDIO_VOLUME_SYNC);
> +    virtio_msg_uint32_to_le((uint8_t *)caps, size, 0);
>  
>      vdagent_virtio_port_write(vport, VDP_CLIENT_PORT,
>                                VD_AGENT_ANNOUNCE_CAPABILITIES, 0,
> @@ -151,8 +180,8 @@ static void do_client_monitors(struct vdagent_virtio_port
> *vport, int port_nr,
>                      (uint8_t *)mon_config, size);
>  
>      /* Acknowledge reception of monitors config to spice server / client */
> -    reply.type  = VD_AGENT_MONITORS_CONFIG;
> -    reply.error = VD_AGENT_SUCCESS;
> +    reply.type  = GUINT32_TO_LE(VD_AGENT_MONITORS_CONFIG);
> +    reply.error = GUINT32_TO_LE(VD_AGENT_SUCCESS);
>      vdagent_virtio_port_write(vport, port_nr, VD_AGENT_REPLY, 0,
>                                (uint8_t *)&reply, sizeof(reply));
>  }
> @@ -255,8 +284,8 @@ static void send_file_xfer_status(struct
> vdagent_virtio_port *vport,
>                                    const char *msg, uint32_t id, uint32_t
>                                    xfer_status)
>  {
>      VDAgentFileXferStatusMessage status = {
> -        .id = id,
> -        .result = xfer_status,
> +        .id = GUINT32_TO_LE(id),
> +        .result = GUINT32_TO_LE(xfer_status),
>      };
>      syslog(LOG_WARNING, msg, id);
>      if (vport)
> @@ -323,6 +352,8 @@ static int virtio_port_read_complete(
>          uint8_t *data)
>  {
>      uint32_t min_size = 0;
> +    uint32_t *data_type = (uint32_t *)data;
> +    uint32_t *id = (uint32_t *)data;
>  
>      if (message_header->protocol != VD_AGENT_PROTOCOL) {
>          syslog(LOG_ERR, "message with wrong protocol version ignoring");
> @@ -333,6 +364,7 @@ static int virtio_port_read_complete(
>      case VD_AGENT_MOUSE_STATE:
>          if (message_header->size != sizeof(VDAgentMouseState))
>              goto size_error;
> +        virtio_msg_uint32_from_le(data, message_header->size, 0);
>          vdagentd_uinput_do_mouse(&uinput, (VDAgentMouseState *)data);
>          if (!uinput) {
>              /* Try to re-open the tablet */
> @@ -356,12 +388,14 @@ static int virtio_port_read_complete(
>      case VD_AGENT_MONITORS_CONFIG:
>          if (message_header->size < sizeof(VDAgentMonitorsConfig))
>              goto size_error;
> +        virtio_msg_uint32_from_le(data, message_header->size, 0);
>          do_client_monitors(vport, port_nr, message_header,
>                      (VDAgentMonitorsConfig *)data);
>          break;
>      case VD_AGENT_ANNOUNCE_CAPABILITIES:
>          if (message_header->size < sizeof(VDAgentAnnounceCapabilities))
>              goto size_error;
> +        virtio_msg_uint32_from_le(data, message_header->size, 0);
>          do_client_capabilities(vport, message_header,
>                          (VDAgentAnnounceCapabilities *)data);
>          break;
> @@ -380,15 +414,41 @@ static int virtio_port_read_complete(
>          if (VD_AGENT_HAS_CAPABILITY(capabilities, capabilities_size,
>                                      VD_AGENT_CAP_CLIPBOARD_SELECTION)) {
>              min_size += 4;
> +            data_type++;
>          }
>          if (message_header->size < min_size) {
>              goto size_error;
>          }
> +        switch (message_header->type) {
> +        case VD_AGENT_CLIPBOARD_REQUEST:
> +        case VD_AGENT_CLIPBOARD:
> +            *data_type = GUINT32_FROM_LE(*data_type);
> +            break;
> +        case VD_AGENT_CLIPBOARD_GRAB:
> +            virtio_msg_uint32_from_le(data, message_header->size, min_size);
> +            break;
> +        }
>          do_client_clipboard(vport, message_header, data);
>          break;
>      case VD_AGENT_FILE_XFER_START:
>      case VD_AGENT_FILE_XFER_STATUS:
>      case VD_AGENT_FILE_XFER_DATA:
> +        if (message_header->size < sizeof(VDAgentFileXferStartMessage))
> +            goto size_error;
> +        *id = GUINT32_FROM_LE(*id);
> +        id++; /* size/status */
> +        switch (message_header->type) {
> +        case VD_AGENT_FILE_XFER_DATA:
> +           if (message_header->size < sizeof(VDAgentFileXferDataMessage))
> +               goto size_error;
> +           *((uint64_t *)id) = le64toh(*((uint64_t *)id)); /* size */

Why not GUINT64_FROM_LE ?
There could be portability issues if id is not 64-bit aligned here.
Would be better to use VDAgentFileXferDataMessage structure which
has the right attributes to support misalignment.

> +           break;
> +        case VD_AGENT_FILE_XFER_STATUS:
> +           if (message_header->size < sizeof(VDAgentFileXferStatusMessage))
> +               goto size_error;
> +           *id = GUINT32_FROM_LE(*id); /* status */
> +           break;
> +        }
>          do_client_file_xfer(vport, message_header, data);
>          break;
>      case VD_AGENT_CLIENT_DISCONNECTED:
> @@ -399,15 +459,17 @@ static int virtio_port_read_complete(
>          if (message_header->size != sizeof(VDAgentMaxClipboard))
>              goto size_error;
>          VDAgentMaxClipboard *msg = (VDAgentMaxClipboard *)data;
> -        syslog(LOG_DEBUG, "Set max clipboard: %d", msg->max);
> -        max_clipboard = msg->max;
> +        syslog(LOG_DEBUG, "Set max clipboard: %d",
> GUINT32_FROM_LE(msg->max));
> +        max_clipboard = GUINT32_FROM_LE(msg->max);
>          break;
>      case VD_AGENT_AUDIO_VOLUME_SYNC:
>          if (message_header->size < sizeof(VDAgentAudioVolumeSync))
>              goto size_error;
> +        VDAgentAudioVolumeSync *vdata = (VDAgentAudioVolumeSync *)data;
> +        virtio_msg_uint16_from_le((uint8_t *)vdata, message_header->size,
> +            offsetof(VDAgentAudioVolumeSync, volume));
>  
> -        do_client_volume_sync(vport, port_nr, message_header,
> -                (VDAgentAudioVolumeSync *)data);
> +        do_client_volume_sync(vport, port_nr, message_header, vdata);
>          break;
>      default:
>          syslog(LOG_WARNING, "unknown message type %d, ignoring",
> @@ -423,7 +485,7 @@ size_error:
>  }
>  
>  static void virtio_write_clipboard(uint8_t selection, uint32_t msg_type,
> -    uint32_t data_type, const uint8_t *data, uint32_t data_size)
> +    uint32_t data_type, uint8_t *data, uint32_t data_size)
>  {
>      uint32_t size = data_size;
>  
> @@ -444,15 +506,18 @@ static void virtio_write_clipboard(uint8_t selection,
> uint32_t msg_type,
>          vdagent_virtio_port_write_append(virtio_port, sel, 4);
>      }
>      if (data_type != -1) {
> +        data_type = GUINT32_TO_LE(data_type);
>          vdagent_virtio_port_write_append(virtio_port, (uint8_t*)&data_type,
>          4);
>      }
>  
> +    if (msg_type == VD_AGENT_CLIPBOARD_GRAB)
> +        virtio_msg_uint32_to_le(data, data_size, 0);
>      vdagent_virtio_port_write_append(virtio_port, data, data_size);
>  }
>  
>  /* vdagentd <-> vdagent communication handling */
>  static int do_agent_clipboard(struct udscs_connection *conn,
> -        struct udscs_message_header *header, const uint8_t *data)
> +        struct udscs_message_header *header, uint8_t *data)
>  {
>      uint8_t selection = header->arg1;
>      uint32_t msg_type = 0, data_type = -1, size = header->size;
> @@ -763,8 +828,8 @@ static void agent_read_complete(struct udscs_connection
> **connp,
>          break;
>      case VDAGENTD_FILE_XFER_STATUS:{
>          VDAgentFileXferStatusMessage status;
> -        status.id = header->arg1;
> -        status.result = header->arg2;
> +        status.id = GUINT32_TO_LE(header->arg1);
> +        status.result = GUINT32_TO_LE(header->arg2);
>          vdagent_virtio_port_write(virtio_port, VDP_CLIENT_PORT,
>                                    VD_AGENT_FILE_XFER_STATUS, 0,
>                                    (uint8_t *)&status, sizeof(status));
> diff --git a/src/vdagentd/virtio-port.c b/src/vdagentd/virtio-port.c
> index cedda4d..25838fe 100644
> --- a/src/vdagentd/virtio-port.c
> +++ b/src/vdagentd/virtio-port.c
> @@ -28,6 +28,7 @@
>  #include <sys/select.h>
>  #include <sys/socket.h>
>  #include <sys/un.h>
> +#include <glib.h>
>  
>  #include "virtio-port.h"
>  
> @@ -216,16 +217,16 @@ int vdagent_virtio_port_write_start(
>          return -1;
>      }
>  
> -    chunk_header.port = port_nr;
> -    chunk_header.size = sizeof(message_header) + data_size;
> +    chunk_header.port = GUINT32_TO_LE(port_nr);
> +    chunk_header.size = GUINT32_TO_LE(sizeof(message_header) + data_size);
>      memcpy(new_wbuf->buf + new_wbuf->write_pos, &chunk_header,
>             sizeof(chunk_header));
>      new_wbuf->write_pos += sizeof(chunk_header);
>  
> -    message_header.protocol = VD_AGENT_PROTOCOL;
> -    message_header.type = message_type;
> -    message_header.opaque = message_opaque;
> -    message_header.size = data_size;
> +    message_header.protocol = GUINT32_TO_LE(VD_AGENT_PROTOCOL);
> +    message_header.type = GUINT32_TO_LE(message_type);
> +    message_header.opaque = htole64(message_opaque);
> +    message_header.size = GUINT32_TO_LE(data_size);
>      memcpy(new_wbuf->buf + new_wbuf->write_pos, &message_header,
>             sizeof(message_header));
>      new_wbuf->write_pos += sizeof(message_header);
> @@ -309,13 +310,20 @@ static void vdagent_virtio_port_do_chunk(struct
> vdagent_virtio_port **vportp)
>          memcpy((uint8_t *)&port->message_header + port->message_header_read,
>                 vport->chunk_data, read);
>          port->message_header_read += read;
> -        if (port->message_header_read == sizeof(port->message_header) &&
> -                port->message_header.size) {
> -            port->message_data = malloc(port->message_header.size);
> -            if (!port->message_data) {
> -                syslog(LOG_ERR, "out of memory, disconnecting virtio");
> -                vdagent_virtio_port_destroy(vportp);
> -                return;
> +        if (port->message_header_read == sizeof(port->message_header)) {
> +
> +            port->message_header.protocol =
> GUINT32_FROM_LE(port->message_header.protocol);
> +            port->message_header.type =
> GUINT32_FROM_LE(port->message_header.type);
> +            port->message_header.opaque =
> le64toh(port->message_header.opaque);
> +            port->message_header.size =
> GUINT32_FROM_LE(port->message_header.size);
> +
> +            if (port->message_header.size) {
> +                port->message_data = malloc(port->message_header.size);
> +                if (!port->message_data) {
> +                    syslog(LOG_ERR, "out of memory, disconnecting virtio");
> +                    vdagent_virtio_port_destroy(vportp);
> +                    return;
> +                }
>              }
>          }
>          pos = read;
> @@ -420,6 +428,8 @@ static void vdagent_virtio_port_do_read(struct
> vdagent_virtio_port **vportp)
>      if (vport->chunk_header_read < sizeof(vport->chunk_header)) {
>          vport->chunk_header_read += n;
>          if (vport->chunk_header_read == sizeof(vport->chunk_header)) {
> +            vport->chunk_header.size =
> GUINT32_FROM_LE(vport->chunk_header.size);
> +            vport->chunk_header.port =
> GUINT32_FROM_LE(vport->chunk_header.port);
>              if (vport->chunk_header.size > VD_AGENT_MAX_DATA_SIZE) {
>                  syslog(LOG_ERR, "chunk size %u too large",
>                         vport->chunk_header.size);

Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

Reply via email to