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 -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|