qemu/qemu.git/blockdev.c, branch master

block: Fix BDS use after free during shutdown

2025-12-15T16:31:08+00:00

During shutdown, blockdev_close_all_bdrv_states() drops any block node references that are still owned by the monitor (i.e. the user). However, in doing so, it forgot to also remove the node from monitor_bdrv_states (which qmp_blockdev_del() correctly does), which means that later calls of bdrv_first()/bdrv_next() will still return the (now stale) pointer to the node. Usually there is no such call after this point, but in some cases it can happen. In the reported case, there was an ongoing migration, and the migration thread wasn't shut down yet: migration_shutdown() called by qemu_cleanup() doesn't actually wait for the migration to be shut down, but may just move it to MIGRATION_STATUS_CANCELLING. The next time migration_iteration_finish() runs, it sees the status and tries to re-activate all block devices that migration may have previously inactivated. This is where bdrv_first()/bdrv_next() get called and the access to the already freed node happens. It is debatable if migration_shutdown() should really return before migration has settled, but leaving a dangling pointer in the list of monitor-owned block nodes is clearly a bug either way and fixing it solves the immediate problem, so fix it. Cc: qemu-stable@nongnu.org Reported-by: Thomas Huth Signed-off-by: Kevin Wolf Message-ID: <20251215150714.130214-1-kwolf@redhat.com> Reviewed-by: Thomas Huth Tested-by: Thomas Huth Reviewed-by: Stefan Hajnoczi Signed-off-by: Kevin Wolf

block: enable stats-intervals for storage devices

2025-10-29T11:10:09+00:00

This patch allows stats-intervals to be used for storage devices with the -device option. It accepts a list of interval lengths in JSON format. It configures and collects the stats in the BlockBackend layer through the storage device that consumes the BlockBackend. Signed-off-by: Chandan Somani Message-ID: <20251003220039.1336663-1-csomani@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf

block: mark bdrv_inactivate() as GRAPH_RDLOCK and move drain to callers

2025-07-14T13:42:07+00:00

The function bdrv_inactivate() calls bdrv_drain_all_begin(), which needs to be called with the graph unlocked, so either bdrv_inactivate() should be marked as GRAPH_UNLOCKED or the drain needs to be moved to the callers. The caller in qmp_blockdev_set_active() requires that the locked section covers bdrv_find_node() too, so the latter alternative is chosen. Signed-off-by: Fiona Ebner Message-ID: <20250530151125.955508-36-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf

block: drop wrapper for bdrv_set_backing_hd_drained()

2025-07-14T13:41:58+00:00

Nearly all callers (outside of the tests) are already using the _drained() variant of the function. It doesn't seem worth keeping. Simply adapt the remaining callers of bdrv_set_backing_hd() and rename bdrv_set_backing_hd_drained() to bdrv_set_backing_hd(). Signed-off-by: Fiona Ebner Message-ID: <20250530151125.955508-31-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf

blockdev: avoid locking and draining multiple times in external_snapshot_abort()

2025-07-14T13:41:57+00:00

By using the appropriate variants bdrv_set_backing_hd_drained() and bdrv_try_change_aio_context_locked(), there only needs to be a single drained and write-locked section in external_snapshot_abort(). Signed-off-by: Fiona Ebner Message-ID: <20250530151125.955508-30-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf

block: add bdrv_graph_wrlock_drained() convenience wrapper

2025-07-14T13:40:58+00:00

Many write-locked sections are also drained sections. A new bdrv_graph_wrunlock_drained() wrapper around bdrv_graph_wrunlock() is introduced, which will begin a drained section first. A global variable is used so bdrv_graph_wrunlock() knows if it also needs to end such a drained section. Both the aio_poll call in bdrv_graph_wrlock() and the aio_bh_poll() in bdrv_graph_wrunlock() can re-enter a write-locked section. While for the latter, ending the drain could be moved to before the call, the former requires that the variable is a counter and not just a boolean. Since the wrapper calls bdrv_drain_all_begin(), which must be called with the graph unlocked, mark the wrapper as GRAPH_UNLOCKED too. The switch to the new helpers was generated with the following commands and then manually checked: find . -name '*.c' -exec sed -i -z 's/bdrv_drain_all_begin();\n\s*bdrv_graph_wrlock();/bdrv_graph_wrlock_drained();/g' {} ';' find . -name '*.c' -exec sed -i -z 's/bdrv_graph_wrunlock();\n\s*bdrv_drain_all_end();/bdrv_graph_wrunlock();/g' {} ';' Suggested-by: Kevin Wolf Signed-off-by: Fiona Ebner Message-ID: <20250530151125.955508-25-f.ebner@proxmox.com> [kwolf: Removed redundant GRAPH_UNLOCKED] Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf

blockdev: drain while unlocked in external_snapshot_action()

2025-06-04T16:16:34+00:00

This is in preparation to mark bdrv_drained_begin() as GRAPH_UNLOCKED. Signed-off-by: Fiona Ebner Message-ID: <20250530151125.955508-19-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf

blockdev: drain while unlocked in internal_snapshot_action()

2025-06-04T16:16:34+00:00

This is in preparation to mark bdrv_drained_begin() as GRAPH_UNLOCKED. Signed-off-by: Fiona Ebner Message-ID: <20250530151125.955508-18-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf

block: move drain outside of quorum_add_child()

2025-06-04T16:16:34+00:00

This is part of resolving the deadlock mentioned in commit "block: move draining out of bdrv_change_aio_context() and mark GRAPH_RDLOCK". The quorum_add_child() callback runs under the graph lock, so it is not allowed to drain. It is only called as the .bdrv_add_child() callback, which is only called in the bdrv_add_child() function, which also runs under the graph lock. The bdrv_add_child() function is called by qmp_x_blockdev_change(), where a drained section is introduced. Signed-off-by: Fiona Ebner Message-ID: <20250530151125.955508-15-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf

block: move drain outside of bdrv_try_change_aio_context()

2025-06-04T16:16:34+00:00

This is part of resolving the deadlock mentioned in commit "block: move draining out of bdrv_change_aio_context() and mark GRAPH_RDLOCK". Convert the function to a _locked() version that has to be called with the graph lock held and add a convenience wrapper that has to be called with the graph unlocked, which drains and takes the lock itself. Since bdrv_try_change_aio_context() is global state code, the wrapper is too. Callers are adapted to use the appropriate variant, depending on whether the caller already holds the lock. In the test_set_aio_context() unit test, prior drains can be removed, because draining already happens inside the new wrapper. Note that bdrv_attach_child_common_abort(), bdrv_attach_child_common() and bdrv_root_unref_child() hold the graph lock and are not actually allowed to drain either. This will be addressed in the following commits. Functions like qmp_blockdev_mirror() query the nodes to act on before draining and locking. In theory, draining could invalidate those nodes. This kind of issue is not addressed by these commits. Signed-off-by: Fiona Ebner Reviewed-by: Kevin Wolf Message-ID: <20250530151125.955508-10-f.ebner@proxmox.com> Signed-off-by: Kevin Wolf