[Qemu-devel] [PATCH v2 0/1] Add qemu_get_counted_string

[Qemu-devel] [PATCH v2 0/1] Add qemu_get_counted_string

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Move the reading of length first encoded strings into it's
own function.

v2:
  Terminate return buffer at read length rather than expected length

Dr. David Alan Gilbert (1):
  Add qemu_get_counted_string to read a string prefixed by a count byte

 arch_init.c                   |  9 +++++----
 include/migration/qemu-file.h |  3 +++
 migration/qemu-file.c         | 17 +++++++++++++++++
 savevm.c                      | 11 ++++++-----
 4 files changed, 31 insertions(+), 9 deletions(-)

-- 
2.4.1

[Qemu-devel] [PATCH v2 1/1] Add qemu_get_counted_string to read a string prefixed by a count byte

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

and use it in loadvm_state and ram_load.

Where ever it's used, check the return and error if it failed.

Minor: ram_load was using a 257 byte array for its string, the
       maximum length is 255 bytes + 0 terminator, so fix to 256

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 arch_init.c                   |  9 +++++----
 include/migration/qemu-file.h |  3 +++
 migration/qemu-file.c         | 17 +++++++++++++++++
 savevm.c                      | 11 ++++++-----
 4 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 23d3feb..7e97eb1 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1593,13 +1593,14 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
             total_ram_bytes = addr;
             while (!ret && total_ram_bytes) {
                 RAMBlock *block;
-                uint8_t len;
                 char id[256];
                 ram_addr_t length;
 
-                len = qemu_get_byte(f);
-                qemu_get_buffer(f, (uint8_t *)id, len);
-                id[len] = 0;
+                if (!qemu_get_counted_string(f, id)) {
+                    error_report("Failed to read ID string of RAM Block");
+                    ret = -EINVAL;
+                    break;
+                }
                 length = qemu_get_be64(f);
 
                 QLIST_FOREACH_RCU(block, &ram_list.blocks, next) {
diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h
index a01c5b8..318aa1e 100644
--- a/include/migration/qemu-file.h
+++ b/include/migration/qemu-file.h
@@ -312,4 +312,7 @@ static inline void qemu_get_sbe64s(QEMUFile *f, int64_t *pv)
 {
     qemu_get_be64s(f, (uint64_t *)pv);
 }
+
+size_t qemu_get_counted_string(QEMUFile *f, char buf[256]);
+
 #endif
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 2750365..0ef543a 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -585,3 +585,20 @@ int qemu_put_qemu_file(QEMUFile *f_des, QEMUFile *f_src)
     }
     return len;
 }
+
+/*
+ * Get a string whose length is determined by a single preceding byte
+ * A preallocated 256 byte buffer must be passed in.
+ * Returns: len on success and a 0 terminated string in the buffer
+ *          else 0
+ *          (Note a 0 length string will return 0 either way)
+ */
+size_t qemu_get_counted_string(QEMUFile *f, char buf[256])
+{
+    size_t len = qemu_get_byte(f);
+    size_t res = qemu_get_buffer(f, (uint8_t *)buf, len);
+
+    buf[res] = 0;
+
+    return res == len ? res : 0;
+}
diff --git a/savevm.c b/savevm.c
index 3b0e222..c162dfd 100644
--- a/savevm.c
+++ b/savevm.c
@@ -964,8 +964,7 @@ int qemu_loadvm_state(QEMUFile *f)
     while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
         uint32_t instance_id, version_id, section_id;
         SaveStateEntry *se;
-        char idstr[257];
-        int len;
+        char idstr[256];
 
         trace_qemu_loadvm_state_section(section_type);
         switch (section_type) {
@@ -973,9 +972,11 @@ int qemu_loadvm_state(QEMUFile *f)
         case QEMU_VM_SECTION_FULL:
             /* Read section start */
             section_id = qemu_get_be32(f);
-            len = qemu_get_byte(f);
-            qemu_get_buffer(f, (uint8_t *)idstr, len);
-            idstr[len] = 0;
+            if (!qemu_get_counted_string(f, idstr)) {
+                error_report("Unable to read ID string for section %u",
+                            section_id);
+                return -EINVAL;
+            }
             instance_id = qemu_get_be32(f);
             version_id = qemu_get_be32(f);
 
-- 
2.4.1

Re: [Qemu-devel] [PATCH v2 1/1] Add qemu_get_counted_string to read a string prefixed by a count byte

[Amit Shah]

On (Wed) 20 May 2015 [10:37:05], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> and use it in loadvm_state and ram_load.
> 
> Where ever it's used, check the return and error if it failed.
> 
> Minor: ram_load was using a 257 byte array for its string, the
>        maximum length is 255 bytes + 0 terminator, so fix to 256
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Amit Shah <amit.shah@redhat.com>

		Amit