Re: [Qemu-devel] [PATCH] spapr-vty: Fix bad assert() statement

[Thomas Huth]

On 11.11.2016 00:01, David Gibson wrote:
> On Thu, Nov 10, 2016 at 10:06:37AM +0100, Thomas Huth wrote:
>> When using the serial console in the GTK interface of QEMU (and
>> QEMU has been compiled with CONFIG_VTE), it is possible to trigger
>> the assert() statement in vty_receive() in spapr_vty.c by pasting
>> a chunk of text with length > 16 into the QEMU window.
>> Most of the other serial backends seem to simply drop characters
>> that they can not handle, so I think we should also do the same in
>> spapr-vty to fix this issue. And since it is quite ugly when pasted
>> text is chopped after 16 bytes, we also increase the size of the
>> input buffer here so that we can at least handle a couple of text
>> lines.
> 
> Adding Paolo to CC, as qemu-char maintainer.
> 
> So, vastly increasing the buffer like this doesn't seem right - the
> plain 16550 serial driver doesn't maintain a buffer bigger than its
> small internal FIFO (32 characters IIRC) - or a single byte if the
> FIFO is disabled.
> 
> I thought the other side of the char driver was supposed to call
> can_receive() first and not deliver more bytes than we can handle -
> hence the assert.

Right. The real bug seems to be on the front end side:

gd_vc_in() in ui/gtk.c calls qemu_chr_be_write() without checking the
qemu_chr_be_can_write() first (which internally calls the can_receive()
function of the backend).

So we likely need to fix gd_vc_in() to use qemu_chr_be_can_write()
instead ... but I'm currently puzzled whether that can be done at all,
since this seems to be a callback function - and we likely can not
simply loop/yield here, can we?

> That said, I think vty_can_receive() has a bug - looking at other
> drivers, I think it's supposed to return the amount of buffer space
> available, but we're just returning 0 or 1.  Although AFAICT that
> should still work, just inefficiently.

Right, it should return the amount of free space instead.

> If you use a serial port with the same GTK interface, do you get the
> same problem with pastes of >16 characters being truncated?

Yes, I tried the coldfire image from
http://wiki.qemu.org/download/coldfire-test-0.1.tar.bz2 and that even
only accepts 1 byte when doing the copy-n-paste and drops everything
else. So copy-n-paste is currently completely broken there, too.

 Thomas

>> Buglink: https://bugs.launchpad.net/qemu/+bug/1639322

signature.asc
Description: OpenPGP digital signature