| Message ID | 1478808961-27379-1-git-send-email-thuth@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Nov 10, 2016 at 09:16:01PM +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. > > Buglink: https://bugs.launchpad.net/qemu/+bug/1639322 > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > v2: > - Do not increase the buffer size since this breaks migration. > (We should figure out a better solution for the size of the > buffer once 2.8 is released) Applied to ppc-for-2.8. since actually hitting the assert is clearly bad. I still think this smells like a bug on the other side of the chardev, feeding in more characters than the can_receive() hook says can be supplied. > > hw/char/spapr_vty.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/hw/char/spapr_vty.c b/hw/char/spapr_vty.c > index 31822fe..06b9b39 100644 > --- a/hw/char/spapr_vty.c > +++ b/hw/char/spapr_vty.c > @@ -1,4 +1,5 @@ > #include "qemu/osdep.h" > +#include "qemu/error-report.h" > #include "qapi/error.h" > #include "qemu-common.h" > #include "cpu.h" > @@ -37,7 +38,15 @@ static void vty_receive(void *opaque, const uint8_t *buf, int size) > qemu_irq_pulse(spapr_vio_qirq(&dev->sdev)); > } > for (i = 0; i < size; i++) { > - assert((dev->in - dev->out) < VTERM_BUFSIZE); > + if (dev->in - dev->out >= VTERM_BUFSIZE) { > + static bool reported; > + if (!reported) { > + error_report("VTY input buffer exhausted - characters dropped." > + " (input size = %i)", size); > + reported = true; > + } > + break; > + } > dev->buf[dev->in++ % VTERM_BUFSIZE] = buf[i]; > } > }
diff --git a/hw/char/spapr_vty.c b/hw/char/spapr_vty.c index 31822fe..06b9b39 100644 --- a/hw/char/spapr_vty.c +++ b/hw/char/spapr_vty.c @@ -1,4 +1,5 @@ #include "qemu/osdep.h" +#include "qemu/error-report.h" #include "qapi/error.h" #include "qemu-common.h" #include "cpu.h" @@ -37,7 +38,15 @@ static void vty_receive(void *opaque, const uint8_t *buf, int size) qemu_irq_pulse(spapr_vio_qirq(&dev->sdev)); } for (i = 0; i < size; i++) { - assert((dev->in - dev->out) < VTERM_BUFSIZE); + if (dev->in - dev->out >= VTERM_BUFSIZE) { + static bool reported; + if (!reported) { + error_report("VTY input buffer exhausted - characters dropped." + " (input size = %i)", size); + reported = true; + } + break; + } dev->buf[dev->in++ % VTERM_BUFSIZE] = buf[i]; } }
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. Buglink: https://bugs.launchpad.net/qemu/+bug/1639322 Signed-off-by: Thomas Huth <thuth@redhat.com> --- v2: - Do not increase the buffer size since this breaks migration. (We should figure out a better solution for the size of the buffer once 2.8 is released) hw/char/spapr_vty.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)