yeahuh / rpms / qemu-kvm

Forked from rpms/qemu-kvm 2 years ago
Clone

Blame SOURCES/kvm-opts-don-t-silently-truncate-long-option-values.patch

4ec855
From 6abc65aaa666bf41070fa772293982cb0d1ae835 Mon Sep 17 00:00:00 2001
4ec855
From: Laszlo Ersek <lersek@redhat.com>
4ec855
Date: Thu, 12 Sep 2019 13:05:00 +0100
4ec855
Subject: [PATCH 03/22] opts: don't silently truncate long option values
4ec855
MIME-Version: 1.0
4ec855
Content-Type: text/plain; charset=UTF-8
4ec855
Content-Transfer-Encoding: 8bit
4ec855
4ec855
RH-Author: Laszlo Ersek <lersek@redhat.com>
4ec855
Message-id: <20190912130503.14094-4-lersek@redhat.com>
4ec855
Patchwork-id: 90436
4ec855
O-Subject: [RHEL-8.2.0 qemu-kvm PATCH 3/6] opts: don't silently truncate long option values
4ec855
Bugzilla: 1749022
4ec855
RH-Acked-by: Stefano Garzarella <sgarzare@redhat.com>
4ec855
RH-Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>
4ec855
RH-Acked-by: Eduardo Habkost <ehabkost@redhat.com>
4ec855
4ec855
From: Daniel P. Berrangé <berrange@redhat.com>
4ec855
4ec855
The existing QemuOpts parsing code uses a fixed size 1024 byte buffer
4ec855
for storing the option values. If a value exceeded this size it was
4ec855
silently truncated and no error reported to the user. Long option values
4ec855
is not a common scenario, but it is conceivable that they will happen.
4ec855
eg if the user has a very deeply nested filesystem it would be possible
4ec855
to come up with a disk path that was > 1024 bytes. Most of the time if
4ec855
such data was silently truncated, the user would get an error about
4ec855
opening a non-existant disk. If they're unlucky though, QEMU might use a
4ec855
completely different disk image from another VM, which could be
4ec855
considered a security issue. Another example program was in using the
4ec855
-smbios command line arg with very large data blobs. In this case the
4ec855
silent truncation will be providing semantically incorrect data to the
4ec855
guest OS for SMBIOS tables.
4ec855
4ec855
If the operating system didn't limit the user's argv when spawning QEMU,
4ec855
the code should honour whatever length arguments were given without
4ec855
imposing its own length restrictions. This patch thus changes the code
4ec855
to use a heap allocated buffer for storing the values during parsing,
4ec855
lifting the arbitrary length restriction.
4ec855
4ec855
RHEL8 notes:
4ec855
4ec855
- Fix up upstream's obviously garbled UTF8 sequences in Dan's name (Author
4ec855
  meta-datum, Signed-off-by tags).
4ec855
4ec855
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
4ec855
Message-Id: <20180416111743.8473-4-berrange@redhat.com>
4ec855
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
4ec855
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
4ec855
(cherry picked from commit 950c4e6c94b15cd0d8b63891dddd7a8dbf458e6a)
4ec855
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
4ec855
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
4ec855
---
4ec855
 hw/i386/multiboot.c   |  33 +++++++++------
4ec855
 include/qemu/option.h |   2 +-
4ec855
 util/qemu-option.c    | 111 +++++++++++++++++++++++++++-----------------------
4ec855
 3 files changed, 81 insertions(+), 65 deletions(-)
4ec855
4ec855
diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
4ec855
index 5bc0a2c..7a2953e 100644
4ec855
--- a/hw/i386/multiboot.c
4ec855
+++ b/hw/i386/multiboot.c
4ec855
@@ -291,12 +291,16 @@ int load_multiboot(FWCfgState *fw_cfg,
4ec855
     cmdline_len = strlen(kernel_filename) + 1;
4ec855
     cmdline_len += strlen(kernel_cmdline) + 1;
4ec855
     if (initrd_filename) {
4ec855
-        const char *r = initrd_filename;
4ec855
+        const char *r = get_opt_value(initrd_filename, NULL);
4ec855
         cmdline_len += strlen(r) + 1;
4ec855
         mbs.mb_mods_avail = 1;
4ec855
-        while (*(r = get_opt_value(NULL, 0, r))) {
4ec855
-           mbs.mb_mods_avail++;
4ec855
-           r++;
4ec855
+        while (1) {
4ec855
+            mbs.mb_mods_avail++;
4ec855
+            r = get_opt_value(r, NULL);
4ec855
+            if (!*r) {
4ec855
+                break;
4ec855
+            }
4ec855
+            r++;
4ec855
         }
4ec855
     }
4ec855
 
4ec855
@@ -313,7 +317,8 @@ int load_multiboot(FWCfgState *fw_cfg,
4ec855
 
4ec855
     if (initrd_filename) {
4ec855
         const char *next_initrd;
4ec855
-        char not_last, tmpbuf[strlen(initrd_filename) + 1];
4ec855
+        char not_last;
4ec855
+        char *one_file = NULL;
4ec855
 
4ec855
         mbs.offset_mods = mbs.mb_buf_size;
4ec855
 
4ec855
@@ -322,24 +327,26 @@ int load_multiboot(FWCfgState *fw_cfg,
4ec855
             int mb_mod_length;
4ec855
             uint32_t offs = mbs.mb_buf_size;
4ec855
 
4ec855
-            next_initrd = get_opt_value(tmpbuf, sizeof(tmpbuf), initrd_filename);
4ec855
+            next_initrd = get_opt_value(initrd_filename, &one_file);
4ec855
             not_last = *next_initrd;
4ec855
             /* if a space comes after the module filename, treat everything
4ec855
                after that as parameters */
4ec855
-            hwaddr c = mb_add_cmdline(&mbs, tmpbuf);
4ec855
-            if ((next_space = strchr(tmpbuf, ' ')))
4ec855
+            hwaddr c = mb_add_cmdline(&mbs, one_file);
4ec855
+            next_space = strchr(one_file, ' ');
4ec855
+            if (next_space) {
4ec855
                 *next_space = '\0';
4ec855
-            mb_debug("multiboot loading module: %s", tmpbuf);
4ec855
-            mb_mod_length = get_image_size(tmpbuf);
4ec855
+            }
4ec855
+            mb_debug("multiboot loading module: %s", one_file);
4ec855
+            mb_mod_length = get_image_size(one_file);
4ec855
             if (mb_mod_length < 0) {
4ec855
-                error_report("Failed to open file '%s'", tmpbuf);
4ec855
+                error_report("Failed to open file '%s'", one_file);
4ec855
                 exit(1);
4ec855
             }
4ec855
 
4ec855
             mbs.mb_buf_size = TARGET_PAGE_ALIGN(mb_mod_length + mbs.mb_buf_size);
4ec855
             mbs.mb_buf = g_realloc(mbs.mb_buf, mbs.mb_buf_size);
4ec855
 
4ec855
-            load_image(tmpbuf, (unsigned char *)mbs.mb_buf + offs);
4ec855
+            load_image(one_file, (unsigned char *)mbs.mb_buf + offs);
4ec855
             mb_add_mod(&mbs, mbs.mb_buf_phys + offs,
4ec855
                        mbs.mb_buf_phys + offs + mb_mod_length, c);
4ec855
 
4ec855
@@ -347,6 +354,8 @@ int load_multiboot(FWCfgState *fw_cfg,
4ec855
                      (char *)mbs.mb_buf + offs,
4ec855
                      (char *)mbs.mb_buf + offs + mb_mod_length, c);
4ec855
             initrd_filename = next_initrd+1;
4ec855
+            g_free(one_file);
4ec855
+            one_file = NULL;
4ec855
         } while (not_last);
4ec855
     }
4ec855
 
4ec855
diff --git a/include/qemu/option.h b/include/qemu/option.h
4ec855
index 1cfe5cb..3dfb449 100644
4ec855
--- a/include/qemu/option.h
4ec855
+++ b/include/qemu/option.h
4ec855
@@ -28,7 +28,7 @@
4ec855
 
4ec855
 #include "qemu/queue.h"
4ec855
 
4ec855
-const char *get_opt_value(char *buf, int buf_size, const char *p);
4ec855
+const char *get_opt_value(const char *p, char **value);
4ec855
 
4ec855
 void parse_option_size(const char *name, const char *value,
4ec855
                        uint64_t *ret, Error **errp);
4ec855
diff --git a/util/qemu-option.c b/util/qemu-option.c
4ec855
index b99568f..ba44a08 100644
4ec855
--- a/util/qemu-option.c
4ec855
+++ b/util/qemu-option.c
4ec855
@@ -70,25 +70,37 @@ static const char *get_opt_name(const char *p, char **option, char delim)
4ec855
  * delimiter is fixed to be comma which starts a new option. To specify an
4ec855
  * option value that contains commas, double each comma.
4ec855
  */
4ec855
-const char *get_opt_value(char *buf, int buf_size, const char *p)
4ec855
+const char *get_opt_value(const char *p, char **value)
4ec855
 {
4ec855
-    char *q;
4ec855
+    size_t capacity = 0, length;
4ec855
+    const char *offset;
4ec855
+
4ec855
+    *value = NULL;
4ec855
+    while (1) {
4ec855
+        offset = strchr(p, ',');
4ec855
+        if (!offset) {
4ec855
+            offset = p + strlen(p);
4ec855
+        }
4ec855
 
4ec855
-    q = buf;
4ec855
-    while (*p != '\0') {
4ec855
-        if (*p == ',') {
4ec855
-            if (*(p + 1) != ',')
4ec855
-                break;
4ec855
-            p++;
4ec855
+        length = offset - p;
4ec855
+        if (*offset != '\0' && *(offset + 1) == ',') {
4ec855
+            length++;
4ec855
+        }
4ec855
+        if (value) {
4ec855
+            *value = g_renew(char, *value, capacity + length + 1);
4ec855
+            strncpy(*value + capacity, p, length);
4ec855
+            (*value)[capacity + length] = '\0';
4ec855
+        }
4ec855
+        capacity += length;
4ec855
+        if (*offset == '\0' ||
4ec855
+            *(offset + 1) != ',') {
4ec855
+            break;
4ec855
         }
4ec855
-        if (q && (q - buf) < buf_size - 1)
4ec855
-            *q++ = *p;
4ec855
-        p++;
4ec855
+
4ec855
+        p += (offset - p) + 2;
4ec855
     }
4ec855
-    if (q)
4ec855
-        *q = '\0';
4ec855
 
4ec855
-    return p;
4ec855
+    return offset;
4ec855
 }
4ec855
 
4ec855
 static void parse_option_bool(const char *name, const char *value, bool *ret,
4ec855
@@ -162,50 +174,43 @@ void parse_option_size(const char *name, const char *value,
4ec855
 
4ec855
 bool has_help_option(const char *param)
4ec855
 {
4ec855
-    size_t buflen = strlen(param) + 1;
4ec855
-    char *buf = g_malloc(buflen);
4ec855
     const char *p = param;
4ec855
     bool result = false;
4ec855
 
4ec855
-    while (*p) {
4ec855
-        p = get_opt_value(buf, buflen, p);
4ec855
+    while (*p && !result) {
4ec855
+        char *value;
4ec855
+
4ec855
+        p = get_opt_value(p, &value);
4ec855
         if (*p) {
4ec855
             p++;
4ec855
         }
4ec855
 
4ec855
-        if (is_help_option(buf)) {
4ec855
-            result = true;
4ec855
-            goto out;
4ec855
-        }
4ec855
+        result = is_help_option(value);
4ec855
+        g_free(value);
4ec855
     }
4ec855
 
4ec855
-out:
4ec855
-    g_free(buf);
4ec855
     return result;
4ec855
 }
4ec855
 
4ec855
-bool is_valid_option_list(const char *param)
4ec855
+bool is_valid_option_list(const char *p)
4ec855
 {
4ec855
-    size_t buflen = strlen(param) + 1;
4ec855
-    char *buf = g_malloc(buflen);
4ec855
-    const char *p = param;
4ec855
-    bool result = true;
4ec855
+    char *value = NULL;
4ec855
+    bool result = false;
4ec855
 
4ec855
     while (*p) {
4ec855
-        p = get_opt_value(buf, buflen, p);
4ec855
-        if (*p && !*++p) {
4ec855
-            result = false;
4ec855
+        p = get_opt_value(p, &value);
4ec855
+        if ((*p && !*++p) ||
4ec855
+            (!*value || *value == ',')) {
4ec855
             goto out;
4ec855
         }
4ec855
 
4ec855
-        if (!*buf || *buf == ',') {
4ec855
-            result = false;
4ec855
-            goto out;
4ec855
-        }
4ec855
+        g_free(value);
4ec855
+        value = NULL;
4ec855
     }
4ec855
 
4ec855
+    result = true;
4ec855
 out:
4ec855
-    g_free(buf);
4ec855
+    g_free(value);
4ec855
     return result;
4ec855
 }
4ec855
 
4ec855
@@ -486,7 +491,7 @@ int qemu_opt_unset(QemuOpts *opts, const char *name)
4ec855
     }
4ec855
 }
4ec855
 
4ec855
-static void opt_set(QemuOpts *opts, const char *name, const char *value,
4ec855
+static void opt_set(QemuOpts *opts, const char *name, char *value,
4ec855
                     bool prepend, Error **errp)
4ec855
 {
4ec855
     QemuOpt *opt;
4ec855
@@ -495,6 +500,7 @@ static void opt_set(QemuOpts *opts, const char *name, const char *value,
4ec855
 
4ec855
     desc = find_desc_by_name(opts->list->desc, name);
4ec855
     if (!desc && !opts_accepts_any(opts)) {
4ec855
+        g_free(value);
4ec855
         error_setg(errp, QERR_INVALID_PARAMETER, name);
4ec855
         return;
4ec855
     }
4ec855
@@ -508,8 +514,7 @@ static void opt_set(QemuOpts *opts, const char *name, const char *value,
4ec855
         QTAILQ_INSERT_TAIL(&opts->head, opt, next);
4ec855
     }
4ec855
     opt->desc = desc;
4ec855
-    opt->str = g_strdup(value);
4ec855
-    assert(opt->str);
4ec855
+    opt->str = value;
4ec855
     qemu_opt_parse(opt, &local_err);
4ec855
     if (local_err) {
4ec855
         error_propagate(errp, local_err);
4ec855
@@ -520,7 +525,7 @@ static void opt_set(QemuOpts *opts, const char *name, const char *value,
4ec855
 void qemu_opt_set(QemuOpts *opts, const char *name, const char *value,
4ec855
                   Error **errp)
4ec855
 {
4ec855
-    opt_set(opts, name, value, false, errp);
4ec855
+    opt_set(opts, name, g_strdup(value), false, errp);
4ec855
 }
4ec855
 
4ec855
 void qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val,
4ec855
@@ -754,7 +759,7 @@ static void opts_do_parse(QemuOpts *opts, const char *params,
4ec855
                           const char *firstname, bool prepend, Error **errp)
4ec855
 {
4ec855
     char *option = NULL;
4ec855
-    char value[1024];
4ec855
+    char *value = NULL;
4ec855
     const char *p,*pe,*pc;
4ec855
     Error *local_err = NULL;
4ec855
 
4ec855
@@ -766,15 +771,15 @@ static void opts_do_parse(QemuOpts *opts, const char *params,
4ec855
             if (p == params && firstname) {
4ec855
                 /* implicitly named first option */
4ec855
                 option = g_strdup(firstname);
4ec855
-                p = get_opt_value(value, sizeof(value), p);
4ec855
+                p = get_opt_value(p, &value);
4ec855
             } else {
4ec855
                 /* option without value, probably a flag */
4ec855
                 p = get_opt_name(p, &option, ',');
4ec855
                 if (strncmp(option, "no", 2) == 0) {
4ec855
                     memmove(option, option+2, strlen(option+2)+1);
4ec855
-                    pstrcpy(value, sizeof(value), "off");
4ec855
+                    value = g_strdup("off");
4ec855
                 } else {
4ec855
-                    pstrcpy(value, sizeof(value), "on");
4ec855
+                    value = g_strdup("on");
4ec855
                 }
4ec855
             }
4ec855
         } else {
4ec855
@@ -782,11 +787,12 @@ static void opts_do_parse(QemuOpts *opts, const char *params,
4ec855
             p = get_opt_name(p, &option, '=');
4ec855
             assert(*p == '=');
4ec855
             p++;
4ec855
-            p = get_opt_value(value, sizeof(value), p);
4ec855
+            p = get_opt_value(p, &value);
4ec855
         }
4ec855
         if (strcmp(option, "id") != 0) {
4ec855
             /* store and parse */
4ec855
             opt_set(opts, option, value, prepend, &local_err);
4ec855
+            value = NULL;
4ec855
             if (local_err) {
4ec855
                 error_propagate(errp, local_err);
4ec855
                 goto cleanup;
4ec855
@@ -796,11 +802,13 @@ static void opts_do_parse(QemuOpts *opts, const char *params,
4ec855
             break;
4ec855
         }
4ec855
         g_free(option);
4ec855
-        option = NULL;
4ec855
+        g_free(value);
4ec855
+        option = value = NULL;
4ec855
     }
4ec855
 
4ec855
  cleanup:
4ec855
     g_free(option);
4ec855
+    g_free(value);
4ec855
 }
4ec855
 
4ec855
 /**
4ec855
@@ -819,7 +827,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
4ec855
                             bool permit_abbrev, bool defaults, Error **errp)
4ec855
 {
4ec855
     const char *firstname;
4ec855
-    char value[1024], *id = NULL;
4ec855
+    char *id = NULL;
4ec855
     const char *p;
4ec855
     QemuOpts *opts;
4ec855
     Error *local_err = NULL;
4ec855
@@ -828,11 +836,9 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
4ec855
     firstname = permit_abbrev ? list->implied_opt_name : NULL;
4ec855
 
4ec855
     if (strncmp(params, "id=", 3) == 0) {
4ec855
-        get_opt_value(value, sizeof(value), params+3);
4ec855
-        id = value;
4ec855
+        get_opt_value(params + 3, &id;;
4ec855
     } else if ((p = strstr(params, ",id=")) != NULL) {
4ec855
-        get_opt_value(value, sizeof(value), p+4);
4ec855
-        id = value;
4ec855
+        get_opt_value(p + 4, &id;;
4ec855
     }
4ec855
 
4ec855
     /*
4ec855
@@ -844,6 +850,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
4ec855
      */
4ec855
     assert(!defaults || list->merge_lists);
4ec855
     opts = qemu_opts_create(list, id, !defaults, &local_err);
4ec855
+    g_free(id);
4ec855
     if (opts == NULL) {
4ec855
         error_propagate(errp, local_err);
4ec855
         return NULL;
4ec855
-- 
4ec855
1.8.3.1
4ec855