Re: [Spice-devel] [PATCH spice 4/8 v2] Receive the GraphicsDeviceInfo message from the streaming agent
On Wed, 2019-01-16 at 13:34 +0100, Lukáš Hrázký wrote:
> > > +StreamMsgDeviceDisplayInfo *display_info_msg =
> > > &dev->msg->device_display_info;
> > > +
> > > +size_t device_address_len =
> > > GUINT32_FROM_LE(display_info_msg->device_address_len);
> > > +if (device_address_len > MAX_DEVICE_ADDRESS_LEN) {
> > > +g_error("Received a device address longer than %u (%zu),
> > > "
> > > +"will be truncated!", MAX_DEVICE_ADDRESS_LEN,
> > > device_address_len);
> >
> > DoS, g_error will abort() Qemu.
>
> I'll put a warning there, though I think it should be logged as an
> error...
Well, it's an error to send an address that is too long. I don't think
it can be considered an error to *receive* such a message. So I think
warning is perfectly fine here. We need to handle invalid input, after
all.
Jonathon
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH spice 4/8 v2] Receive the GraphicsDeviceInfo message from the streaming agent
On Tue, 2019-01-15 at 08:31 -0500, Frediano Ziglio wrote:
> >
> > Receives the GraphicsDeviceInfo message from the streaming agent and
> > stores the data in a list on the streaming device.
> >
> > Signed-off-by: Lukáš Hrázký
> > ---
> > server/display-limits.h| 3 ++
> > server/red-qxl.c | 2 +-
> > server/red-stream-device.c | 67 --
> > server/red-stream-device.h | 8 +
> > server/reds.c | 1 +
> > 5 files changed, 78 insertions(+), 3 deletions(-)
> >
> > diff --git a/server/display-limits.h b/server/display-limits.h
> > index e875149b..d79d3211 100644
> > --- a/server/display-limits.h
> > +++ b/server/display-limits.h
> > @@ -25,4 +25,7 @@
> > /** Maximum number of streams created by spice-server */
> > #define NUM_STREAMS 50
> >
> > +/** Maximum length of the device address string */
> > +#define MAX_DEVICE_ADDRESS_LEN 256
> > +
> > #endif /* DISPLAY_LIMITS_H_ */
> > diff --git a/server/red-qxl.c b/server/red-qxl.c
> > index a56d9a52..943ccb08 100644
> > --- a/server/red-qxl.c
> > +++ b/server/red-qxl.c
> > @@ -37,11 +37,11 @@
> > #include "dispatcher.h"
> > #include "red-parse-qxl.h"
> > #include "red-channel-client.h"
> > +#include "display-limits.h"
> >
> > #include "red-qxl.h"
> >
> >
> > -#define MAX_DEVICE_ADDRESS_LEN 256
> > #define MAX_MONITORS_COUNT 16
> >
> > struct QXLState {
> > diff --git a/server/red-stream-device.c b/server/red-stream-device.c
> > index 215ddbe7..20bf115d 100644
> > --- a/server/red-stream-device.c
> > +++ b/server/red-stream-device.c
> > @@ -37,6 +37,7 @@ struct StreamDevice {
> > StreamMsgCapabilities capabilities;
> > StreamMsgCursorSet cursor_set;
> > StreamMsgCursorMove cursor_move;
> > +StreamMsgDeviceDisplayInfo device_display_info;
> > uint8_t buf[STREAM_MSG_CAPABILITIES_MAX_BYTES];
> > } *msg;
> > uint32_t msg_pos;
> > @@ -49,6 +50,7 @@ struct StreamDevice {
> > CursorChannel *cursor_channel;
> > SpiceTimer *close_timer;
> > uint32_t frame_mmtime;
> > +StreamDeviceDisplayInfo device_display_info;
> > };
> >
> > struct StreamDeviceClass {
> > @@ -64,8 +66,8 @@ static void char_device_set_state(RedCharDevice *char_dev,
> > int state);
> > typedef bool StreamMsgHandler(StreamDevice *dev, SpiceCharDeviceInstance
> > *sin)
> > SPICE_GNUC_WARN_UNUSED_RESULT;
> >
> > -static StreamMsgHandler handle_msg_format, handle_msg_data,
> > handle_msg_cursor_set,
> > -handle_msg_cursor_move, handle_msg_capabilities;
> > +static StreamMsgHandler handle_msg_format, handle_msg_device_display_info,
> > handle_msg_data,
> > +handle_msg_cursor_set, handle_msg_cursor_move, handle_msg_capabilities;
> >
> > static bool handle_msg_invalid(StreamDevice *dev, SpiceCharDeviceInstance
> > *sin,
> > const char *error_msg)
> > SPICE_GNUC_WARN_UNUSED_RESULT;
> > @@ -146,6 +148,13 @@ stream_device_partial_read(StreamDevice *dev,
> > SpiceCharDeviceInstance *sin)
> > handled = handle_msg_format(dev, sin);
> > }
> > break;
> > +case STREAM_TYPE_DEVICE_DISPLAY_INFO:
> > +if (dev->hdr.size > sizeof(StreamMsgDeviceDisplayInfo) +
> > MAX_DEVICE_ADDRESS_LEN) {
> > +handled = handle_msg_invalid(dev, sin,
> > "StreamMsgDeviceDisplayInfo too large");
> > +} else {
> > +handled = handle_msg_device_display_info(dev, sin);
> > +}
> > +break;
> > case STREAM_TYPE_DATA:
> > if (dev->hdr.size > 32*1024*1024) {
> > handled = handle_msg_invalid(dev, sin, "STREAM_DATA too
> > large");
> > @@ -271,6 +280,60 @@ handle_msg_format(StreamDevice *dev,
> > SpiceCharDeviceInstance *sin)
> > return true;
> > }
> >
> > +static bool
> > +handle_msg_device_display_info(StreamDevice *dev, SpiceCharDeviceInstance
> > *sin)
> > +{
> > +SpiceCharDeviceInterface *sif = spice_char_device_get_interface(sin);
> > +
> > +if (spice_extra_checks) {
> > +spice_assert(dev->hdr_pos >= sizeof(StreamDevHeader));
> > +spice_assert(dev->hdr.type == STREAM_TYPE_DEVICE_DISPLAY_INFO);
> > +}
> > +
> > +if (dev->msg_len < dev->hdr.size) {
> > +dev->msg = g_realloc(dev->msg, dev->hdr.size);
> > +dev->msg_len = dev->hdr.size;
> > +}
> > +
> > +/* read from device */
> > +ssize_t n = sif->read(sin, dev->msg->buf + dev->msg_pos, dev->hdr.size
> > -
> > dev->msg_pos);
> > +if (n <= 0) {
> > +return dev->msg_pos == dev->hdr.size;
> > +}
> > +
> > +dev->msg_pos += n;
> > +if (dev->msg_pos != dev->hdr.size) { /* some bytes are still missing */
> > +return false;
> > +}
> > +
> > +StreamMsgDeviceDisplayInfo *display_info_msg =
> > &dev->msg->device_display_info;
> > +
> > +size_t device_address_len =
> > GUINT32_FROM_LE(display_info_msg->device_address_len);
> > +if (devic
Re: [Spice-devel] [PATCH spice 4/8 v2] Receive the GraphicsDeviceInfo message from the streaming agent
>
> Receives the GraphicsDeviceInfo message from the streaming agent and
> stores the data in a list on the streaming device.
>
> Signed-off-by: Lukáš Hrázký
> ---
> server/display-limits.h| 3 ++
> server/red-qxl.c | 2 +-
> server/red-stream-device.c | 67 --
> server/red-stream-device.h | 8 +
> server/reds.c | 1 +
> 5 files changed, 78 insertions(+), 3 deletions(-)
>
> diff --git a/server/display-limits.h b/server/display-limits.h
> index e875149b..d79d3211 100644
> --- a/server/display-limits.h
> +++ b/server/display-limits.h
> @@ -25,4 +25,7 @@
> /** Maximum number of streams created by spice-server */
> #define NUM_STREAMS 50
>
> +/** Maximum length of the device address string */
> +#define MAX_DEVICE_ADDRESS_LEN 256
> +
> #endif /* DISPLAY_LIMITS_H_ */
> diff --git a/server/red-qxl.c b/server/red-qxl.c
> index a56d9a52..943ccb08 100644
> --- a/server/red-qxl.c
> +++ b/server/red-qxl.c
> @@ -37,11 +37,11 @@
> #include "dispatcher.h"
> #include "red-parse-qxl.h"
> #include "red-channel-client.h"
> +#include "display-limits.h"
>
> #include "red-qxl.h"
>
>
> -#define MAX_DEVICE_ADDRESS_LEN 256
> #define MAX_MONITORS_COUNT 16
>
> struct QXLState {
> diff --git a/server/red-stream-device.c b/server/red-stream-device.c
> index 215ddbe7..20bf115d 100644
> --- a/server/red-stream-device.c
> +++ b/server/red-stream-device.c
> @@ -37,6 +37,7 @@ struct StreamDevice {
> StreamMsgCapabilities capabilities;
> StreamMsgCursorSet cursor_set;
> StreamMsgCursorMove cursor_move;
> +StreamMsgDeviceDisplayInfo device_display_info;
> uint8_t buf[STREAM_MSG_CAPABILITIES_MAX_BYTES];
> } *msg;
> uint32_t msg_pos;
> @@ -49,6 +50,7 @@ struct StreamDevice {
> CursorChannel *cursor_channel;
> SpiceTimer *close_timer;
> uint32_t frame_mmtime;
> +StreamDeviceDisplayInfo device_display_info;
> };
>
> struct StreamDeviceClass {
> @@ -64,8 +66,8 @@ static void char_device_set_state(RedCharDevice *char_dev,
> int state);
> typedef bool StreamMsgHandler(StreamDevice *dev, SpiceCharDeviceInstance
> *sin)
> SPICE_GNUC_WARN_UNUSED_RESULT;
>
> -static StreamMsgHandler handle_msg_format, handle_msg_data,
> handle_msg_cursor_set,
> -handle_msg_cursor_move, handle_msg_capabilities;
> +static StreamMsgHandler handle_msg_format, handle_msg_device_display_info,
> handle_msg_data,
> +handle_msg_cursor_set, handle_msg_cursor_move, handle_msg_capabilities;
>
> static bool handle_msg_invalid(StreamDevice *dev, SpiceCharDeviceInstance
> *sin,
> const char *error_msg)
> SPICE_GNUC_WARN_UNUSED_RESULT;
> @@ -146,6 +148,13 @@ stream_device_partial_read(StreamDevice *dev,
> SpiceCharDeviceInstance *sin)
> handled = handle_msg_format(dev, sin);
> }
> break;
> +case STREAM_TYPE_DEVICE_DISPLAY_INFO:
> +if (dev->hdr.size > sizeof(StreamMsgDeviceDisplayInfo) +
> MAX_DEVICE_ADDRESS_LEN) {
> +handled = handle_msg_invalid(dev, sin,
> "StreamMsgDeviceDisplayInfo too large");
> +} else {
> +handled = handle_msg_device_display_info(dev, sin);
> +}
> +break;
> case STREAM_TYPE_DATA:
> if (dev->hdr.size > 32*1024*1024) {
> handled = handle_msg_invalid(dev, sin, "STREAM_DATA too large");
> @@ -271,6 +280,60 @@ handle_msg_format(StreamDevice *dev,
> SpiceCharDeviceInstance *sin)
> return true;
> }
>
> +static bool
> +handle_msg_device_display_info(StreamDevice *dev, SpiceCharDeviceInstance
> *sin)
> +{
> +SpiceCharDeviceInterface *sif = spice_char_device_get_interface(sin);
> +
> +if (spice_extra_checks) {
> +spice_assert(dev->hdr_pos >= sizeof(StreamDevHeader));
> +spice_assert(dev->hdr.type == STREAM_TYPE_DEVICE_DISPLAY_INFO);
> +}
> +
> +if (dev->msg_len < dev->hdr.size) {
> +dev->msg = g_realloc(dev->msg, dev->hdr.size);
> +dev->msg_len = dev->hdr.size;
> +}
> +
> +/* read from device */
> +ssize_t n = sif->read(sin, dev->msg->buf + dev->msg_pos, dev->hdr.size -
> dev->msg_pos);
> +if (n <= 0) {
> +return dev->msg_pos == dev->hdr.size;
> +}
> +
> +dev->msg_pos += n;
> +if (dev->msg_pos != dev->hdr.size) { /* some bytes are still missing */
> +return false;
> +}
> +
> +StreamMsgDeviceDisplayInfo *display_info_msg =
> &dev->msg->device_display_info;
> +
> +size_t device_address_len =
> GUINT32_FROM_LE(display_info_msg->device_address_len);
> +if (device_address_len > MAX_DEVICE_ADDRESS_LEN) {
> +g_error("Received a device address longer than %u (%zu), "
> +"will be truncated!", MAX_DEVICE_ADDRESS_LEN,
> device_address_len);
DoS, g_error will abort() Qemu.
> +device_address_len =
> sizeof(dev->device_display_info.device_address);
> +}
> +
> +s
[Spice-devel] [PATCH spice 4/8 v2] Receive the GraphicsDeviceInfo message from the streaming agent
Receives the GraphicsDeviceInfo message from the streaming agent and
stores the data in a list on the streaming device.
Signed-off-by: Lukáš Hrázký
---
server/display-limits.h| 3 ++
server/red-qxl.c | 2 +-
server/red-stream-device.c | 67 --
server/red-stream-device.h | 8 +
server/reds.c | 1 +
5 files changed, 78 insertions(+), 3 deletions(-)
diff --git a/server/display-limits.h b/server/display-limits.h
index e875149b..d79d3211 100644
--- a/server/display-limits.h
+++ b/server/display-limits.h
@@ -25,4 +25,7 @@
/** Maximum number of streams created by spice-server */
#define NUM_STREAMS 50
+/** Maximum length of the device address string */
+#define MAX_DEVICE_ADDRESS_LEN 256
+
#endif /* DISPLAY_LIMITS_H_ */
diff --git a/server/red-qxl.c b/server/red-qxl.c
index a56d9a52..943ccb08 100644
--- a/server/red-qxl.c
+++ b/server/red-qxl.c
@@ -37,11 +37,11 @@
#include "dispatcher.h"
#include "red-parse-qxl.h"
#include "red-channel-client.h"
+#include "display-limits.h"
#include "red-qxl.h"
-#define MAX_DEVICE_ADDRESS_LEN 256
#define MAX_MONITORS_COUNT 16
struct QXLState {
diff --git a/server/red-stream-device.c b/server/red-stream-device.c
index 215ddbe7..20bf115d 100644
--- a/server/red-stream-device.c
+++ b/server/red-stream-device.c
@@ -37,6 +37,7 @@ struct StreamDevice {
StreamMsgCapabilities capabilities;
StreamMsgCursorSet cursor_set;
StreamMsgCursorMove cursor_move;
+StreamMsgDeviceDisplayInfo device_display_info;
uint8_t buf[STREAM_MSG_CAPABILITIES_MAX_BYTES];
} *msg;
uint32_t msg_pos;
@@ -49,6 +50,7 @@ struct StreamDevice {
CursorChannel *cursor_channel;
SpiceTimer *close_timer;
uint32_t frame_mmtime;
+StreamDeviceDisplayInfo device_display_info;
};
struct StreamDeviceClass {
@@ -64,8 +66,8 @@ static void char_device_set_state(RedCharDevice *char_dev,
int state);
typedef bool StreamMsgHandler(StreamDevice *dev, SpiceCharDeviceInstance *sin)
SPICE_GNUC_WARN_UNUSED_RESULT;
-static StreamMsgHandler handle_msg_format, handle_msg_data,
handle_msg_cursor_set,
-handle_msg_cursor_move, handle_msg_capabilities;
+static StreamMsgHandler handle_msg_format, handle_msg_device_display_info,
handle_msg_data,
+handle_msg_cursor_set, handle_msg_cursor_move, handle_msg_capabilities;
static bool handle_msg_invalid(StreamDevice *dev, SpiceCharDeviceInstance *sin,
const char *error_msg)
SPICE_GNUC_WARN_UNUSED_RESULT;
@@ -146,6 +148,13 @@ stream_device_partial_read(StreamDevice *dev,
SpiceCharDeviceInstance *sin)
handled = handle_msg_format(dev, sin);
}
break;
+case STREAM_TYPE_DEVICE_DISPLAY_INFO:
+if (dev->hdr.size > sizeof(StreamMsgDeviceDisplayInfo) +
MAX_DEVICE_ADDRESS_LEN) {
+handled = handle_msg_invalid(dev, sin, "StreamMsgDeviceDisplayInfo
too large");
+} else {
+handled = handle_msg_device_display_info(dev, sin);
+}
+break;
case STREAM_TYPE_DATA:
if (dev->hdr.size > 32*1024*1024) {
handled = handle_msg_invalid(dev, sin, "STREAM_DATA too large");
@@ -271,6 +280,60 @@ handle_msg_format(StreamDevice *dev,
SpiceCharDeviceInstance *sin)
return true;
}
+static bool
+handle_msg_device_display_info(StreamDevice *dev, SpiceCharDeviceInstance *sin)
+{
+SpiceCharDeviceInterface *sif = spice_char_device_get_interface(sin);
+
+if (spice_extra_checks) {
+spice_assert(dev->hdr_pos >= sizeof(StreamDevHeader));
+spice_assert(dev->hdr.type == STREAM_TYPE_DEVICE_DISPLAY_INFO);
+}
+
+if (dev->msg_len < dev->hdr.size) {
+dev->msg = g_realloc(dev->msg, dev->hdr.size);
+dev->msg_len = dev->hdr.size;
+}
+
+/* read from device */
+ssize_t n = sif->read(sin, dev->msg->buf + dev->msg_pos, dev->hdr.size -
dev->msg_pos);
+if (n <= 0) {
+return dev->msg_pos == dev->hdr.size;
+}
+
+dev->msg_pos += n;
+if (dev->msg_pos != dev->hdr.size) { /* some bytes are still missing */
+return false;
+}
+
+StreamMsgDeviceDisplayInfo *display_info_msg =
&dev->msg->device_display_info;
+
+size_t device_address_len =
GUINT32_FROM_LE(display_info_msg->device_address_len);
+if (device_address_len > MAX_DEVICE_ADDRESS_LEN) {
+g_error("Received a device address longer than %u (%zu), "
+"will be truncated!", MAX_DEVICE_ADDRESS_LEN,
device_address_len);
+device_address_len = sizeof(dev->device_display_info.device_address);
+}
+
+strncpy(dev->device_display_info.device_address,
+(char*) display_info_msg->device_address,
+device_address_len);
+
+// make sure the string is terminated
+dev->device_display_info.device_address[device_address_len - 1] = '\0';
+
+dev->device_display_info.stream_id =
GUINT32_FROM_LE(display_info_msg->strea
