qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, richard.henderson@linaro.org, qemu-devel@nongnu.org
Subject: [PULL 17/18] aio-posix: do not nest poll handlers
Date: Wed, 17 May 2023 18:51:15 +0200	[thread overview]
Message-ID: <20230517165116.475123-18-kwolf@redhat.com> (raw)
In-Reply-To: <20230517165116.475123-1-kwolf@redhat.com>

From: Stefan Hajnoczi <stefanha@redhat.com>

QEMU's event loop supports nesting, which means that event handler
functions may themselves call aio_poll(). The condition that triggered a
handler must be reset before the nested aio_poll() call, otherwise the
same handler will be called and immediately re-enter aio_poll. This
leads to an infinite loop and stack exhaustion.

Poll handlers are especially prone to this issue, because they typically
reset their condition by finishing the processing of pending work.
Unfortunately it is during the processing of pending work that nested
aio_poll() calls typically occur and the condition has not yet been
reset.

Disable a poll handler during ->io_poll_ready() so that a nested
aio_poll() call cannot invoke ->io_poll_ready() again. As a result, the
disabled poll handler and its associated fd handler do not run during
the nested aio_poll(). Calling aio_set_fd_handler() from inside nested
aio_poll() could cause it to run again. If the fd handler is pending
inside nested aio_poll(), then it will also run again.

In theory fd handlers can be affected by the same issue, but they are
more likely to reset the condition before calling nested aio_poll().

This is a special case and it's somewhat complex, but I don't see a way
around it as long as nested aio_poll() is supported.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2186181
Fixes: c38270692593 ("block: Mark bdrv_co_io_(un)plug() and callers GRAPH_RDLOCK")
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20230502184134.534703-2-stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 util/aio-posix.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/util/aio-posix.c b/util/aio-posix.c
index a8be940f76..34bc2a64d8 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -353,8 +353,19 @@ static bool aio_dispatch_handler(AioContext *ctx, AioHandler *node)
         poll_ready && revents == 0 &&
         aio_node_check(ctx, node->is_external) &&
         node->io_poll_ready) {
+        /*
+         * Remove temporarily to avoid infinite loops when ->io_poll_ready()
+         * calls aio_poll() before clearing the condition that made the poll
+         * handler become ready.
+         */
+        QLIST_SAFE_REMOVE(node, node_poll);
+
         node->io_poll_ready(node->opaque);
 
+        if (!QLIST_IS_INSERTED(node, node_poll)) {
+            QLIST_INSERT_HEAD(&ctx->poll_aio_handlers, node, node_poll);
+        }
+
         /*
          * Return early since revents was zero. aio_notify() does not count as
          * progress.
-- 
2.40.1



  parent reply	other threads:[~2023-05-17 16:52 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-17 16:50 [PULL 00/18] Block layer patches Kevin Wolf
2023-05-17 16:50 ` [PULL 01/18] blockdev: refactor transaction to use Transaction API Kevin Wolf
2023-05-17 16:51 ` [PULL 02/18] blockdev: transactions: rename some things Kevin Wolf
2023-05-17 16:51 ` [PULL 03/18] blockdev: qmp_transaction: refactor loop to classic for Kevin Wolf
2023-05-17 16:51 ` [PULL 04/18] blockdev: transaction: refactor handling transaction properties Kevin Wolf
2023-05-17 16:51 ` [PULL 05/18] blockdev: use state.bitmap in block-dirty-bitmap-add action Kevin Wolf
2023-05-17 16:51 ` [PULL 06/18] blockdev: qmp_transaction: drop extra generic layer Kevin Wolf
2023-05-17 16:51 ` [PULL 07/18] docs/interop/qcow2.txt: fix description about "zlib" clusters Kevin Wolf
2023-05-17 16:51 ` [PULL 08/18] block: Call .bdrv_co_create(_opts) unlocked Kevin Wolf
2023-05-17 16:51 ` [PULL 09/18] block/export: Fix null pointer dereference in error path Kevin Wolf
2023-05-17 16:51 ` [PULL 10/18] qcow2: Unlock the graph in qcow2_do_open() where necessary Kevin Wolf
2023-05-17 16:51 ` [PULL 11/18] qemu-img: Take graph lock more selectively Kevin Wolf
2023-05-17 16:51 ` [PULL 12/18] test-bdrv-drain: " Kevin Wolf
2023-05-17 16:51 ` [PULL 13/18] test-bdrv-drain: Call bdrv_co_unref() in coroutine context Kevin Wolf
2023-05-17 16:51 ` [PULL 14/18] blockjob: Adhere to rate limit even when reentered early Kevin Wolf
2023-05-17 16:51 ` [PULL 15/18] graph-lock: Honour read locks even in the main thread Kevin Wolf
2023-05-17 16:51 ` [PULL 16/18] iotests/245: Check if 'compress' driver is available Kevin Wolf
2023-05-17 16:51 ` Kevin Wolf [this message]
2023-05-18  7:13   ` [PULL 17/18] aio-posix: do not nest poll handlers Michael Tokarev
2023-05-18 15:05     ` Stefan Hajnoczi
2023-05-17 16:51 ` [PULL 18/18] tested: add test for nested aio_poll() in " Kevin Wolf
2023-05-17 19:10   ` Richard Henderson
2023-05-19  9:23     ` Kevin Wolf
2023-05-23 15:36       ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230517165116.475123-18-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).