From 5923ea67b1da641e1d7c3a4d7d59574a57a9816f Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange" <berrange@redhat.com>
Date: Mon, 23 Sep 2013 14:16:09 +0100
Subject: [PATCH] Fix error checking of qemuParseKeywords return status

Most callers of qemuParseKeywords were assigning its return
value to a 'size_t' variable. Then then also checked '< 0'
for error condition, but this will never be true with the
unsigned size_t variable. Rather than using 'ssize_t', change
qemuParseKeywords so that the element count is returned via
an output parameter, leaving the return value solely as an
error indicator.

This avoids a crash accessing beyond the end of an error
upon OOM.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 src/qemu/qemu_command.c      | 33 ++++++++++++++++++++-------------
 src/qemu/qemu_command.h      |  1 +
 src/qemu/qemu_monitor_json.c |  4 +---
 3 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4c55b085c7..83597ee785 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9776,6 +9776,7 @@ int
 qemuParseKeywords(const char *str,
                   char ***retkeywords,
                   char ***retvalues,
+                  int *retnkeywords,
                   int allowEmptyValue)
 {
     int keywordCount = 0;
@@ -9788,6 +9789,7 @@ qemuParseKeywords(const char *str,
 
     *retkeywords = NULL;
     *retvalues = NULL;
+    *retnkeywords = 0;
     end = start + strlen(str);
 
     while (start) {
@@ -9857,8 +9859,8 @@ qemuParseKeywords(const char *str,
 
     *retkeywords = keywords;
     *retvalues = values;
-
-    return keywordCount;
+    *retnkeywords = keywordCount;
+    return 0;
 
 error:
     for (i = 0; i < keywordCount; i++) {
@@ -9893,9 +9895,11 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt,
     int unitid = -1;
     int trans = VIR_DOMAIN_DISK_TRANS_DEFAULT;
 
-    if ((nkeywords = qemuParseKeywords(val,
-                                       &keywords,
-                                       &values, 0)) < 0)
+    if (qemuParseKeywords(val,
+                          &keywords,
+                          &values,
+                          &nkeywords,
+                          0) < 0)
         return NULL;
 
     if (VIR_ALLOC(def) < 0)
@@ -10244,9 +10248,11 @@ qemuParseCommandLineNet(virDomainXMLOptionPtr xmlopt,
     tmp = strchr(val, ',');
 
     if (tmp) {
-        if ((nkeywords = qemuParseKeywords(tmp+1,
-                                           &keywords,
-                                           &values, 0)) < 0)
+        if (qemuParseKeywords(tmp+1,
+                              &keywords,
+                              &values,
+                              &nkeywords,
+                              0) < 0)
             return NULL;
     } else {
         nkeywords = 0;
@@ -10314,9 +10320,11 @@ qemuParseCommandLineNet(virDomainXMLOptionPtr xmlopt,
     VIR_FREE(values);
 
     if (STRPREFIX(nic, "nic,")) {
-        if ((nkeywords = qemuParseKeywords(nic + strlen("nic,"),
-                                           &keywords,
-                                           &values, 0)) < 0) {
+        if (qemuParseKeywords(nic + strlen("nic,"),
+                              &keywords,
+                              &values,
+                              &nkeywords,
+                              0) < 0) {
             virDomainNetDefFree(def);
             def = NULL;
             goto cleanup;
@@ -10820,8 +10828,7 @@ qemuParseCommandLineSmp(virDomainDefPtr dom,
     char *end;
     int ret;
 
-    nkws = qemuParseKeywords(val, &kws, &vals, 1);
-    if (nkws < 0)
+    if (qemuParseKeywords(val, &kws, &vals, &nkws, 1) < 0)
         return -1;
 
     for (i = 0; i < nkws; i++) {
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 2f248ebbc2..0e16a3d4d5 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -296,6 +296,7 @@ int
 qemuParseKeywords(const char *str,
                   char ***retkeywords,
                   char ***retvalues,
+                  int *retnkeywords,
                   int allowEmptyValue);
 
 #endif /* __QEMU_COMMAND_H__*/
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 12f7e69115..2d841616d2 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -533,9 +533,7 @@ qemuMonitorJSONKeywordStringToJSON(const char *str, const char *firstkeyword)
     if (!(ret = virJSONValueNewObject()))
         goto error;
 
-    nkeywords = qemuParseKeywords(str, &keywords, &values, 1);
-
-    if (nkeywords < 0)
+    if (qemuParseKeywords(str, &keywords, &values, &nkeywords, 1) < 0)
         goto error;
 
     for (i = 0; i < nkeywords; i++) {
-- 
GitLab