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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
