Re: [PATCH spice-gtk] Run-time check monitor per display count <= 256

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Marc-Andre,

On 07/17/2012 08:11 PM, Marc-André Lureau wrote:
Limit range of monitors, to avoid potential crashes lead by invalid
received MonitorConfig values (could be misconfigured or misbehaving
guest)

This is a a client-side implementation limitation. Eventually, the
range could be inscreased (or unlimited == 0) in the future...
---
  gtk/channel-display.c |   14 +++++++++++++-
  1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/gtk/channel-display.c b/gtk/channel-display.c
index d77447a..345d26e 100644
--- a/gtk/channel-display.c
+++ b/gtk/channel-display.c
@@ -275,7 +275,7 @@ static void spice_display_channel_class_init(SpiceDisplayChannelClass *klass)
           g_param_spec_uint("monitors-max",
                             "Max display monitors",
                             "The current maximum number of monitors",
-                           1, G_MAXINT16, 1,
+                           1, 256, 1,

A good idea is to have 256 as a constant.

                             G_PARAM_READABLE |
                             G_PARAM_STATIC_STRINGS));

@@ -1493,6 +1493,8 @@ static void display_handle_surface_destroy(SpiceChannel *channel, SpiceMsgIn *in
      free(surface);
  }

+#define CLAMP_CHECK(x, low, high)  (((x)>  (high)) ? TRUE : (((x)<  (low)) ? TRUE : FALSE))
+
  /* coroutine context */
  static void display_handle_monitors_config(SpiceChannel *channel, SpiceMsgIn *in)
  {
@@ -1506,6 +1508,16 @@ static void display_handle_monitors_config(SpiceChannel *channel, SpiceMsgIn *in
      SPICE_DEBUG("monitors config: n: %d/%d", config->count, config->max_allowed);

      c->monitors_max = config->max_allowed;
+    if (CLAMP_CHECK(c->monitors_max, 1, 256)) {
+        g_warning("MonitorConfig max_allowed is not within permitted range, clamping");
+        c->monitors_max = CLAMP(c->monitors_max, 1, 256);

I'm not sure it's a good idea to continue and process the message once it was
found to have an invalid number for monitors_max.
For example if we that value is 1024, CLAMP would make it 256, and the
loop below would fill 256 entries with possibly bad values.

+    }
+
+    if (CLAMP_CHECK(config->count, 1, c->monitors_max)) {
+        g_warning("MonitorConfig count is not within permitted range, clamping");
+        config->count = CLAMP(config->count, 1, c->monitors_max);
+    }
+
      c->monitors = g_array_set_size(c->monitors, config->count);

      for (i = 0; i<  config->count; i++) {

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



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]