qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] chardev/char-io: Fix polling by not removing polls when buffers


From: Iris Johnson
Subject: [PATCH] chardev/char-io: Fix polling by not removing polls when buffers are full
Date: Fri, 29 Jan 2021 19:56:31 +0000

Currently, the chardev backend code will prepare for IO polling to occur
by potentially adding or removing a watch of the backing channel for the
chardev. The chardev poll is added if the fd_can_read() function reports
more than 0 byte of buffer space, if a poll handler is already setup and
the bufer is now empty, the poll handler is removed.

This causes a bug where the device buffer becomes ready, but the poll is
blocking on a sleep (potentially forever), because the buffer is small
and fills up immediately, while the backend channel has more data. This
leads to a stall condition or potentially a deadlock in the guest.

The guest is looping, waiting for data to be reported as ready to read,
the host sees that the buffer is ready for reading and adds the poll,
the poll returns since data is available and data is made available to
the guest. Before the guest code is able to retrieve the data and clear
the full buffer, the poll code runs again, sees that the buffer is now
full, and removes the poll. At this point only a timeout from another
polled source, or another source having it's poll complete will result
in the loop running again to see that the buffer is now ready and to
add the poll again.

We solve this issue by removing the logic that removes the poll, keeping
the existing logic to only create the poll once there's space for the
first read.

Buglink: https://bugs.launchpad.net/qemu/+bug/1913341
Signed-off-by: Iris Johnson <iris@modwiz.com>
---
 chardev/char-io.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/chardev/char-io.c b/chardev/char-io.c
index 8ced184160..fa9e222f78 100644
--- a/chardev/char-io.c
+++ b/chardev/char-io.c
@@ -50,16 +50,14 @@ static gboolean io_watch_poll_prepare(GSource *source,
         return FALSE;
     }
 
-    if (now_active) {
+    if (now_active && !was_active) {
         iwp->src = qio_channel_create_watch(
             iwp->ioc, G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL);
         g_source_set_callback(iwp->src, iwp->fd_read, iwp->opaque, NULL);
         g_source_add_child_source(source, iwp->src);
         g_source_unref(iwp->src);
-    } else {
-        g_source_remove_child_source(source, iwp->src);
-        iwp->src = NULL;
     }
+
     return FALSE;
 }
 
-- 
2.25.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]