[Spice-devel] [PATCH spice-server] StreamDevice: Handle incomplete reads of StreamMsgFormat

[Christophe de Dinechin]

Jonathon Jongsma writes:

> This is currently unlikely to happen since we communicate over a pipe
> and the pipe buffer is sufficiently large to avoid splitting the
> message. But for completeness, we should handle this scenario.
>
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>

Reviewed-by: Christophe de Dinechin <dinechin at redhat.com>

> ---
>  server/stream-device.c | 33 +++++++++++++++++++++++----------
>  1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/server/stream-device.c b/server/stream-device.c
> index fc5b50659..259527246 100644
> --- a/server/stream-device.c
> +++ b/server/stream-device.c
> @@ -42,6 +42,12 @@ struct StreamDevice {
>
>      StreamDevHeader hdr;
>      uint8_t hdr_pos;
> +    union {
> +        StreamMsgFormat format;
> +        StreamMsgCapabilities capabilities;
> +        uint8_t buf[1024];
> +    } msg;
> +    uint8_t msg_pos;
>      bool has_error;
>      bool opened;
>      bool flow_stopped;
> @@ -155,20 +161,27 @@ handle_msg_invalid(StreamDevice *dev, SpiceCharDeviceInstance *sin, const char *
>  static bool
>  handle_msg_format(StreamDevice *dev, SpiceCharDeviceInstance *sin)
>  {
> -    StreamMsgFormat fmt;
>      SpiceCharDeviceInterface *sif = spice_char_device_get_interface(sin);
> -    int n = sif->read(sin, (uint8_t *) &fmt, sizeof(fmt));
> -    if (n == 0) {
> -        return false;
> -    }
> -    if (n != sizeof(fmt)) {
> +
> +    spice_assert(dev->hdr_pos >= sizeof(StreamDevHeader));
> +    spice_assert(dev->hdr.type == STREAM_TYPE_FORMAT);
> +
> +    int n = sif->read(sin, dev->msg.buf + dev->msg_pos, sizeof(StreamMsgFormat) - dev->msg_pos);
> +    if (n < 0) {
>          return handle_msg_invalid(dev, sin, NULL);
>      }
> -    fmt.width = GUINT32_FROM_LE(fmt.width);
> -    fmt.height = GUINT32_FROM_LE(fmt.height);
> -    stream_channel_change_format(dev->stream_channel, &fmt);
>
> -    return true;
> +    dev->msg_pos += n;
> +
> +    if (dev->msg_pos >= sizeof(StreamMsgFormat)) {
> +        dev->msg.format.width = GUINT32_FROM_LE(dev->msg.format.width);
> +        dev->msg.format.height = GUINT32_FROM_LE(dev->msg.format.height);
> +        stream_channel_change_format(dev->stream_channel, &dev->msg.format);
> +        dev->msg_pos = 0;
> +        return true;
> +    }
> +
> +    return false;
>  }
>
>  static bool


--
Cheers,
Christophe de Dinechin (c3d)