This service will be undergoing maintenance at 00:00 UTC, 2016-08-01. It is expected to last about 1 hours
Bug 580449 - QMP: bad input may cause server to stop respoding
QMP: bad input may cause server to stop respoding
Status: CLOSED DUPLICATE of bug 870311
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: qemu-kvm (Show other bugs)
7.0
All Linux
low Severity low
: rc
: ---
Assigned To: Laszlo Ersek
Virtualization Bugs
:
: 684110 912219 (view as bug list)
Depends On: 617082
Blocks: 559201 580953 580954
  Show dependency treegraph
 
Reported: 2010-04-08 06:00 EDT by juzhang
Modified: 2013-11-13 11:34 EST (History)
19 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-11-13 11:34:14 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description juzhang 2010-04-08 06:00:58 EDT
Description of problem:
Boot a VM with -qmp tcp:0:4444,server , and {"execute": "capabilities" or {"execute": "qpm_capabilities}.qmp Server does not respond.after that,input correct format command,qmp Server  still does not respond.

Version-Release number of selected component (if applicable):
qemu-kvm-0.12.1.2-2.35.el6.x86_64
kernel: 
2.6.32-19.el6.x86_64


How reproducible:
100%

Steps to Reproduce:
1./usr/libexec/qemu-kvm  -no-hpet -usbdevice tablet -rtc-td-hack -m 4G -smp 8
-drive file=/root/zhangjunyi/rhel6_64.qcow2,if=ide,boot=on -net
nic,vlan=0,macaddr=22:11:22:45:66:99,model=e1000 -net
tap,vlan=0,script=/etc/qemu-ifup -uuid `uuidgen` -cpu qemu64,+sse2 -balloon
virtio -boot c -monitor stdio -qmp tcp:0:4444,server  -vnc :10
2.#telnet 10.66.91.52 4444
3.{"execute": "capabilities" or {"execute": "qpm_capabilities}
4.after step3,input correct format command,qmp Server  still does not respond.
{"execute": "qpm_capabilities"}

Actual results:
qmp Server does not respond when input wrong format commands in qmp client.after that,even input correct format commands, qmp Server still does not respond.

Expected results:
qmp Server should respond whatever input wrong format commands or correct format commands.If input wrong format commands,need a QMP event notification like "{"error": {"class": "JSONParsing", "desc": "Invalid JSON syntax", "data": {}}}"
Comment 2 RHEL Product and Program Management 2010-04-08 06:59:17 EDT
This request was evaluated by Red Hat Product Management for inclusion in a Red
Hat Enterprise Linux major release.  Product Management has requested further
review of this request by Red Hat Engineering, for potential inclusion in a Red
Hat Enterprise Linux Major release.  This request is not yet committed for
inclusion.
Comment 3 Suqin Huang 2010-05-14 01:05:58 EDT
input unknow_cmd mixed with _, no response for the command
{"execute":yy_uu}
Comment 4 Luiz Capitulino 2010-05-14 10:14:56 EDT
Looking into this issue, should fix it soon.
Comment 5 Luiz Capitulino 2010-05-21 09:10:48 EDT
FYI: Not all bad inputs cause this, technically what happens is that we handle errors from the JSON parser but we don't handle errors from the JSON lexer.

So, an input like:

{ "execute": xxxxx }

Gets handled as expected, but your input:

{ "execute": _ }

Doesn't. The best solution is probably to just kill the connection.
Comment 7 Luiz Capitulino 2010-07-02 16:20:08 EDT
Reverting Status and Devel Whiteboard, as the patches posted for this one fix a different BZ. This one is still an open issue.
Comment 8 Luiz Capitulino 2010-07-05 14:46:16 EDT
The fix for bug #585009 has changed this behavior. Now, instead of stop responding the server just ignores bad characters, so things like this just
works:

  { "execute": ___"query-block" }

This makes me think this is like bug #586233 and maybe deserves the same destiny (ie. be moved to rhel6.1).

Paolo, you've said you were going to work on this, any update? Can I assign this to you?


For reference, here goes the two possible ways to fix this:

1. Just drop the connection

When the lexer sees a bad character, it returns an error but the monitor code doesn't do anything about it (monitor.c:4342). The best solution would be to
just drop the connection.

However, it seems to me that our qemu-char implementation doesn't has an easy way to do this... Possibly, we would have to write the proper interface for it, which is beyond rhel6.

2. Pass an error token to the parser

We can't drop the connection, but we do handle parser errors just fine (monitor.c:4259). So, a different solution would be to change the lexer to pass an error token to the parser and then it could return an error as soon as it sees the error token.

I think this is what Paolo was/is going to do, but I'm not sure whether it's going to be accepted upstream or how easy this is.
Comment 10 Luiz Capitulino 2010-07-06 12:52:25 EDT
Right now we're accepting only blockers for rhel6, as I believe this is not a release blocker (ie. it doesn't prevent libvirt from talking to qemu) we can move it to rhel6.1.
Comment 11 Luiz Capitulino 2010-07-22 12:32:20 EDT
Bug #617082 is possibly related to this one too.
Comment 17 Luiz Capitulino 2011-11-17 07:18:23 EST
*** Bug 684110 has been marked as a duplicate of this bug. ***
Comment 18 Luiz Capitulino 2011-12-09 14:47:53 EST
This won't be fixed for RHEL6 because it's a minor bug, libvirt would have to send broken json to qemu to trigger this.

But this bug exists upstream and will be fixed there in the near future. Will move this BZ to RHEL7.0 so that we can keep track of it.
Comment 19 Paolo Bonzini 2012-03-07 04:05:53 EST
For the first example

{"execute": "qmp_capabilities
{"execute": "qmp_capabilities"}

the only thing we could do is to disallow a newline inside a quoted string.  Newlines in quoted strings are not valid JSON, but we allowed them so far, so it would be a small backwards incompatibility.

For the second example

{"execute": "qmp_capabilities"
{"execute": "qmp_capabilities"}

there are multiple ways to recover.  You could add both a } or a comma before the second open brace.  Hence, I think this is not recoverable.  The only way would be to add some kind of synchronization sequence, for example a NUL character would force the beginning of a new message.
Comment 20 Pavel Hrdina 2012-05-25 09:23:08 EDT
I've tested this bug and here is how it works now:

1. example:

qmp server is waiting for end of string. If you end string on the new line and also end the object like this:

{"execute": "qmp_capabilities
"}

it correctly emit error response:
{"error": {"class": "CommandNotFound", "desc": "The command qmp_capabilities\r\n has not been found", "data": {"name": "qmp_capabilities\r\n"}}}

2. example:

qmp server is waiting for the end of object or another atribute.

Possible way how to handle this issue could be that we allow command could by only on 1 line or we could end command after we receive 2 new lines.
Comment 23 Luiz Capitulino 2013-02-18 08:09:17 EST
*** Bug 912219 has been marked as a duplicate of this bug. ***
Comment 24 Luiz Capitulino 2013-02-18 08:10:35 EST
See bug 912219 for some additional (and sometimes duplicated) examples of what is mentioned in comment 19.
Comment 25 Laszlo Ersek 2013-11-13 11:34:14 EST

*** This bug has been marked as a duplicate of bug 870311 ***

Note You need to log in before you can comment on or make changes to this bug.