qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] socket: websocket refresh of max_size outside of poll
@ 2019-12-05 14:06 Anisse Astier
  2019-12-05 17:07 ` no-reply
  0 siblings, 1 reply; 3+ messages in thread
From: Anisse Astier @ 2019-12-05 14:06 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anisse Astier, Julia Suvorova, Daniel P . Berrangé

Because serial backend readiness isn't checked, the socket frontend (in
websocket mode) would send new characters before previous characters
were consumed. This lead to skipped characters, or worse, SysRq keys
being triggered.

This patch ensures the readable size is refreshed before consuming any
data. Normally, this size is refreshed in the event loop by the glib
prepare io_watch_poll_prepare; but since the websocket reader is a
greedy one to decode the websocket protocol, (whereas tcp one ready
bytes as necessary), there's nothing to read or poll, so the max_size
wouldn't be refreshed.

Buglink: https://bugs.launchpad.net/qemu/+bug/1828608
Signed-off-by: Anisse Astier <aastier@freebox.fr>
---
 chardev/char-socket.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 185fe38dda..5e093e6605 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -505,6 +505,9 @@ static gboolean tcp_chr_read(QIOChannel *chan, GIOCondition cond, void *opaque)
     uint8_t buf[CHR_READ_BUF_LEN];
     int len, size;
 
+    if(s->is_websock)
+        /* Greedy reader does not have event loop refresh by tcp_chr_read_poll */
+        s->max_size = qemu_chr_be_can_write(chr);
     if ((s->state != TCP_CHARDEV_STATE_CONNECTED) ||
         s->max_size <= 0) {
         return TRUE;
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] socket: websocket refresh of max_size outside of poll
  2019-12-05 14:06 [PATCH] socket: websocket refresh of max_size outside of poll Anisse Astier
@ 2019-12-05 17:07 ` no-reply
  2019-12-06  8:50   ` [PATCH v2] " Anisse Astier
  0 siblings, 1 reply; 3+ messages in thread
From: no-reply @ 2019-12-05 17:07 UTC (permalink / raw)
  To: aastier; +Cc: aastier, berrange, jusual, qemu-devel

Patchew URL: https://patchew.org/QEMU/20191205140645.6071-1-aastier@freebox.fr/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH] socket: websocket refresh of max_size outside of poll
Type: series
Message-id: 20191205140645.6071-1-aastier@freebox.fr

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
17c57dd socket: websocket refresh of max_size outside of poll

=== OUTPUT BEGIN ===
ERROR: space required before the open parenthesis '('
#31: FILE: chardev/char-socket.c:508:
+    if(s->is_websock)

WARNING: line over 80 characters
#32: FILE: chardev/char-socket.c:509:
+        /* Greedy reader does not have event loop refresh by tcp_chr_read_poll */

total: 1 errors, 1 warnings, 9 lines checked

Commit 17c57ddf8310 (socket: websocket refresh of max_size outside of poll) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20191205140645.6071-1-aastier@freebox.fr/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH v2] socket: websocket refresh of max_size outside of poll
  2019-12-05 17:07 ` no-reply
@ 2019-12-06  8:50   ` Anisse Astier
  0 siblings, 0 replies; 3+ messages in thread
From: Anisse Astier @ 2019-12-06  8:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anisse Astier, Julia Suvorova, Daniel P . Berrangé

Because serial backend readiness isn't checked, the socket frontend (in
websocket mode) would send new characters before previous characters
were consumed. This lead to skipped characters, or worse, SysRq keys
being triggered.

This patch ensures the readable size is refreshed before consuming any
data. Normally, this size is refreshed in the event loop by the glib
prepare io_watch_poll_prepare calling tcp_chr_read_poll; but since the
websocket reader is a greedy because it needs a buffer to decode the
websocket protocol, (whereas tcp one ready bytes as necessary), there's
nothing to read or poll, so the max_size wouldn't be refreshed.

Buglink: https://bugs.launchpad.net/qemu/+bug/1828608
Signed-off-by: Anisse Astier <aastier@freebox.fr>
---
Changes since v1:
 - style changes

---
 chardev/char-socket.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 185fe38dda..9267ecd813 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -505,6 +505,9 @@ static gboolean tcp_chr_read(QIOChannel *chan, GIOCondition cond, void *opaque)
     uint8_t buf[CHR_READ_BUF_LEN];
     int len, size;
 
+    if (s->is_websock)
+        /* Buffered greedy reader needs max_size refresh */
+        s->max_size = qemu_chr_be_can_write(chr);
     if ((s->state != TCP_CHARDEV_STATE_CONNECTED) ||
         s->max_size <= 0) {
         return TRUE;
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-12-06 14:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-05 14:06 [PATCH] socket: websocket refresh of max_size outside of poll Anisse Astier
2019-12-05 17:07 ` no-reply
2019-12-06  8:50   ` [PATCH v2] " Anisse Astier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).