debian/patches/CVE-2015-3247.patch

   1 From 524eef10c6c6c2f3f30be28c56b8f96adc7901f0 Mon Sep 17 00:00:00 2001
   2 From: Frediano Ziglio <fziglio@redhat.com>
   3 Date: Tue, 9 Jun 2015 08:50:46 +0100
   4 Subject: [PATCH] Avoid race conditions reading monitor configs from guest
   5
   6 For security reasons do not assume guest do not change structures it
   7 pass to Qemu.
   8 Guest could change count field while Qemu is copying QXLMonitorsConfig
   9 structure leading to heap corruption.
  10 This patch avoid it reading count only once.
  11
  12 Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
  13 ---
  14  server/red_worker.c | 46 ++++++++++++++++++++++++++++++++--------------
  15  1 file changed, 32 insertions(+), 14 deletions(-)
  16
  17 --- a/server/red_worker.c
  18 +++ b/server/red_worker.c
  19 @@ -11051,7 +11051,8 @@ static inline void red_monitors_config_i
  20  }
  21
  22  static void worker_update_monitors_config(RedWorker *worker,
  23 -                                          QXLMonitorsConfig *dev_monitors_config)
  24 +                                          QXLMonitorsConfig *dev_monitors_config,
  25 +                                          uint16_t count, uint16_t max_allowed)
  26  {
  27      int heads_size;
  28      MonitorsConfig *monitors_config;
  29 @@ -11060,22 +11061,22 @@ static void worker_update_monitors_confi
  30      monitors_config_decref(worker->monitors_config);
  31
  32      spice_debug("monitors config %d(%d)",
  33 -                dev_monitors_config->count,
  34 -                dev_monitors_config->max_allowed);
  35 -    for (i = 0; i < dev_monitors_config->count; i++) {
  36 +                count,
  37 +                max_allowed);
  38 +    for (i = 0; i < count; i++) {
  39          spice_debug("+%d+%d %dx%d",
  40                      dev_monitors_config->heads[i].x,
  41                      dev_monitors_config->heads[i].y,
  42                      dev_monitors_config->heads[i].width,
  43                      dev_monitors_config->heads[i].height);
  44      }
  45 -    heads_size = dev_monitors_config->count * sizeof(QXLHead);
  46 +    heads_size = count * sizeof(QXLHead);
  47      worker->monitors_config = monitors_config =
  48          spice_malloc(sizeof(*monitors_config) + heads_size);
  49      monitors_config->refs = 1;
  50      monitors_config->worker = worker;
  51 -    monitors_config->count = dev_monitors_config->count;
  52 -    monitors_config->max_allowed = dev_monitors_config->max_allowed;
  53 +    monitors_config->count = count;
  54 +    monitors_config->max_allowed = max_allowed;
  55      memcpy(monitors_config->heads, dev_monitors_config->heads, heads_size);
  56  }
  57
  58 @@ -11459,33 +11460,50 @@ void handle_dev_display_migrate(void *op
  59      red_migrate_display(worker, rcc);
  60  }
  61
  62 +static inline uint32_t qxl_monitors_config_size(uint32_t heads)
  63 +{
  64 +    return sizeof(QXLMonitorsConfig) + sizeof(QXLHead) * heads;
  65 +}
  66 +
  67  static void handle_dev_monitors_config_async(void *opaque, void *payload)
  68  {
  69      RedWorkerMessageMonitorsConfigAsync *msg = payload;
  70      RedWorker *worker = opaque;
  71 -    int min_size = sizeof(QXLMonitorsConfig) + sizeof(QXLHead);
  72      int error;
  73 +    uint16_t count, max_allowed;
  74      QXLMonitorsConfig *dev_monitors_config =
  75          (QXLMonitorsConfig*)get_virt(&worker->mem_slots, msg->monitors_config,
  76 -                                     min_size, msg->group_id, &error);
  77 +                                     qxl_monitors_config_size(1),
  78 +                                     msg->group_id, &error);
  79
  80      if (error) {
  81          /* TODO: raise guest bug (requires added QXL interface) */
  82          return;
  83      }
  84      worker->driver_cap_monitors_config = 1;
  85 -    if (dev_monitors_config->count == 0) {
  86 +    count = dev_monitors_config->count;
  87 +    max_allowed = dev_monitors_config->max_allowed;
  88 +    if (count == 0) {
  89          spice_warning("ignoring an empty monitors config message from driver");
  90          return;
  91      }
  92 -    if (dev_monitors_config->count > dev_monitors_config->max_allowed) {
  93 +    if (count > max_allowed) {
  94          spice_warning("ignoring malformed monitors_config from driver, "
  95                        "count > max_allowed %d > %d",
  96 -                      dev_monitors_config->count,
  97 -                      dev_monitors_config->max_allowed);
  98 +                      count,
  99 +                      max_allowed);
 100 +        return;
 101 +    }
 102 +    /* get pointer again to check virtual size */
 103 +    dev_monitors_config =
 104 +        (QXLMonitorsConfig*)get_virt(&worker->mem_slots, msg->monitors_config,
 105 +                                     qxl_monitors_config_size(count),
 106 +                                     msg->group_id, &error);
 107 +    if (error) {
 108 +        /* TODO: raise guest bug (requires added QXL interface) */
 109          return;
 110      }
 111 -    worker_update_monitors_config(worker, dev_monitors_config);
 112 +    worker_update_monitors_config(worker, dev_monitors_config, count, max_allowed);
 113      red_worker_push_monitors_config(worker);
 114  }
 115