[Qemu-devel] [PATCH for-1.4 v2 01/13] qmp: Fix design bug and read beyond buffer in memchar-write

[Markus Armbruster]

Command memchar-write takes data and size parameter.  Begs the
question what happens when data doesn't match size.

With format base64, qmp_memchar_write() copies the full data argument,
regardless of size argument.

With format utf8, qmp_memchar_write() copies size bytes from data,
happily reading beyond data.  Copies crap from the heap or even
crashes.

Drop the size parameter, and always copy the full data argument.

Signed-off-by: Markus Armbruster <address@hidden>
---
 hmp.c            | 4 +---
 qapi-schema.json | 4 +---
 qemu-char.c      | 8 +++-----
 qmp-commands.hx  | 4 +---
 4 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/hmp.c b/hmp.c
index 1689e6f..9fdf1ce 100644
--- a/hmp.c
+++ b/hmp.c
@@ -664,13 +664,11 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
 
 void hmp_memchar_write(Monitor *mon, const QDict *qdict)
 {
-    uint32_t size;
     const char *chardev = qdict_get_str(qdict, "device");
     const char *data = qdict_get_str(qdict, "data");
     Error *errp = NULL;
 
-    size = strlen(data);
-    qmp_memchar_write(chardev, size, data, false, 0, &errp);
+    qmp_memchar_write(chardev, data, false, 0, &errp);
 
     hmp_handle_error(mon, &errp);
 }
diff --git a/qapi-schema.json b/qapi-schema.json
index cdd8384..9e2cbbd 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -346,8 +346,6 @@
 #
 # @device: the name of the memory char device.
 #
-# @size: the size to write in bytes.
-#
 # @data: the source data write to memchar.
 #
 # @format: #optional the format of the data write to chardev 'memory',
@@ -359,7 +357,7 @@
 # Since: 1.4
 ##
 { 'command': 'memchar-write',
-  'data': {'device': 'str', 'size': 'int', 'data': 'str',
+  'data': {'device': 'str', 'data': 'str',
            '*format': 'DataFormat'} }
 
 ##
diff --git a/qemu-char.c b/qemu-char.c
index ac5d62d..9c1dd13 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2753,9 +2753,8 @@ static bool qemu_is_chr(const CharDriverState *chr, const 
char *filename)
     return strcmp(chr->filename, filename);
 }
 
-void qmp_memchar_write(const char *device, int64_t size,
-                       const char *data, bool has_format,
-                       enum DataFormat format,
+void qmp_memchar_write(const char *device, const char *data,
+                       bool has_format, enum DataFormat format,
                        Error **errp)
 {
     CharDriverState *chr;
@@ -2774,12 +2773,11 @@ void qmp_memchar_write(const char *device, int64_t size,
         return;
     }
 
-    write_count = (gsize)size;
-
     if (has_format && (format == DATA_FORMAT_BASE64)) {
         write_data = g_base64_decode(data, &write_count);
     } else {
         write_data = (uint8_t *)data;
+        write_count = strlen(data);
     }
 
     ret = cirmem_chr_write(chr, write_data, write_count);
diff --git a/qmp-commands.hx b/qmp-commands.hx
index bbb21f3..8468f10 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -467,7 +467,7 @@ EQMP
 
     {
         .name       = "memchar-write",
-        .args_type  = "device:s,size:i,data:s,format:s?",
+        .args_type  = "device:s,data:s,format:s?",
         .mhandler.cmd_new = qmp_marshal_input_memchar_write,
     },
 
@@ -481,7 +481,6 @@ char device.
 Arguments:
 
 - "device": the name of the char device, must be unique (json-string)
-- "size": the memory size, in bytes, should be power of 2 (json-int)
 - "data": the source data write to memory (json-string)
 - "format": the data format write to memory, default is
             utf8. (json-string, optional)
@@ -491,7 +490,6 @@ Example:
 
 -> { "execute": "memchar-write",
                 "arguments": { "device": foo,
-                               "size": 8,
                                "data": "abcdefgh",
                                "format": "utf8" } }
 <- { "return": {} }
-- 
1.7.11.7