[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] util: Fix return value for virJSONValueFromString if it fails



于 2011年03月23日 21:19, Daniel P. Berrange 写道:
On Wed, Mar 23, 2011 at 08:07:25PM +0800, Osier Yang wrote:
Problem:
   "parser.head" is not NULL even if it's free'ed by "virJSONValueFree",
returning "parser.head" when "virJSONValueFromString" fails will cause
unexpected errors (libvirtd will crash sometimes), e.g.
   In function "qemuMonitorJSONArbitraryCommand":

         if (!(cmd = virJSONValueFromString(cmd_str)))
             goto cleanup;

         if (qemuMonitorJSONCommand(mon, cmd,&reply)<  0)
             goto cleanup;

         ......

      cleanup:
         virJSONValueFree(cmd);

   It will continues to send command to monitor even if "virJSONValueFromString"
is failed, and more worse, it trys to free "cmd" again.

   Crash example:
{"error":{"class":"QMPBadInputObject","desc":"Expected 'execute' in QMP input","data":{"expected":"execute"}}}
{"error":{"class":"QMPBadInputObject","desc":"Expected 'execute' in QMP input","data":{"expected":"execute"}}}
error: server closed connection:
error: unable to connect to '/var/run/libvirt/libvirt-sock', libvirtd may need to be started: Connection refused
error: failed to connect to the hypervisor

   This fix is to:
     1) return NULL for failure of "virJSONValueFromString",
     2) and it seems "virJSONValueFree" uses incorrect loop index for type
        of "VIR_JSON_TYPE_OBJECT", fix it together.

* src/util/json.c
---
  src/util/json.c |    7 +++++--
  1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/util/json.c b/src/util/json.c
index f90594c..be47f64 100644
--- a/src/util/json.c
+++ b/src/util/json.c
@@ -65,7 +65,7 @@ void virJSONValueFree(virJSONValuePtr value)

      switch (value->type) {
      case VIR_JSON_TYPE_OBJECT:
-        for (i = 0 ; i<  value->data.array.nvalues ; i++) {
+        for (i = 0 ; i<  value->data.object.npairs; i++) {
              VIR_FREE(value->data.object.pairs[i].key);
              virJSONValueFree(value->data.object.pairs[i].value);
          }
@@ -897,6 +897,7 @@ virJSONValuePtr virJSONValueFromString(const char *jsonstring)
      yajl_parser_config cfg = { 1, 1 };
      yajl_handle hand;
      virJSONParser parser = { NULL, NULL, 0 };
+    virJSONValuePtr ret = NULL;

      VIR_DEBUG("string=%s", jsonstring);

@@ -917,6 +918,8 @@ virJSONValuePtr virJSONValueFromString(const char *jsonstring)
          goto cleanup;
      }

+    ret = parser.head;
+
  cleanup:
      yajl_free(hand);

@@ -930,7 +933,7 @@ cleanup:

      VIR_DEBUG("result=%p", parser.head);

-    return parser.head;
+    return ret;
  }

ACK

Daniel

Thanks, pushed.

Regards
Osier


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]