* [PATCH] block/throttle-groups: fix crash when enabling I/O limits on a busy disk
2026-09-08 14:55 [PATCH 0/1] block/throttle-groups: fix a NULL token crash on group creation Denis V. Lunev
@ 2026-09-08 14:55 ` Denis V. Lunev
2026-09-11 10:52 ` Alberto Garcia
2026-09-21 19:40 ` [PATCH 0/1] block/throttle-groups: fix a NULL token crash on group creation Denis V. Lunev
2026-09-22 14:47 ` Kevin Wolf
2 siblings, 1 reply; 5+ messages in thread
From: Denis V. Lunev @ 2026-09-08 14:55 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, qemu-stable, Denis V. Lunev, Kevin Wolf, Hanna Reitz,
Alberto Garcia
From: Denis V. Lunev <den@openvz.org>
throttle_group_register_tgm() stores tgm->throttle_state before it takes
tg->lock, so the I/O path can see a member whose group still has NULL
tokens[] and an empty member list. A request in an iothread then reaches
throttle_group_co_io_limits_intercept(), wins tg->lock ahead of the
registering thread, and next_throttle_token() passes the NULL token to
throttle_group_next_tgm(), which dereferences it:
throttle_group_next_tgm (tgm=0x0) at block/throttle-groups.c:185
next_throttle_token (tgm=..., direction=THROTTLE_READ)
throttle_group_co_io_limits_intercept (tgm=..., bytes=8192, ...)
blk_co_do_preadv_part (blk=..., offset=..., bytes=8192, ...)
blk_aio_read_entry (opaque=...)
coroutine_trampoline (i0=..., i1=...)
blk_io_limits_enable() does not drain the BlockBackend, unlike its disable
counterpart, so nothing keeps requests away while the group is built. A
guest probing a disk that libvirt has just attached, at the moment the QoS
settings for it are applied, is enough to hit this.
Publish tgm->throttle_state with a release store once the member is
linked into the group and its timers exist, and read it with an acquire
load on the two unlocked I/O paths. A request that sees the new pointer
then also sees a fully built group.
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Hanna Reitz <hreitz@redhat.com>
Cc: Alberto Garcia <berto@igalia.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
block/block-backend.c | 12 ++++++------
block/throttle-groups.c | 4 +++-
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index 37ba7e9fc4..5e59bac1a3 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1338,6 +1338,7 @@ blk_co_do_preadv_part(BlockBackend *blk, int64_t offset, int64_t bytes,
QEMUIOVector *qiov, size_t qiov_offset,
BdrvRequestFlags flags)
{
+ ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
int ret;
BlockDriverState *bs;
IO_CODE();
@@ -1357,9 +1358,8 @@ blk_co_do_preadv_part(BlockBackend *blk, int64_t offset, int64_t bytes,
bdrv_inc_in_flight(bs);
/* throttling disk I/O */
- if (blk->public.throttle_group_member.throttle_state) {
- throttle_group_co_io_limits_intercept(&blk->public.throttle_group_member,
- bytes, THROTTLE_READ);
+ if (qatomic_load_acquire(&tgm->throttle_state)) {
+ throttle_group_co_io_limits_intercept(tgm, bytes, THROTTLE_READ);
}
ret = bdrv_co_preadv_part(blk->root, offset, bytes, qiov, qiov_offset,
@@ -1413,6 +1413,7 @@ blk_co_do_pwritev_part(BlockBackend *blk, int64_t offset, int64_t bytes,
QEMUIOVector *qiov, size_t qiov_offset,
BdrvRequestFlags flags)
{
+ ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
int ret;
BlockDriverState *bs;
IO_CODE();
@@ -1431,9 +1432,8 @@ blk_co_do_pwritev_part(BlockBackend *blk, int64_t offset, int64_t bytes,
bdrv_inc_in_flight(bs);
/* throttling disk I/O */
- if (blk->public.throttle_group_member.throttle_state) {
- throttle_group_co_io_limits_intercept(&blk->public.throttle_group_member,
- bytes, THROTTLE_WRITE);
+ if (qatomic_load_acquire(&tgm->throttle_state)) {
+ throttle_group_co_io_limits_intercept(tgm, bytes, THROTTLE_WRITE);
}
if (!blk->enable_write_cache) {
diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 805e47270c..faf74a969d 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -581,7 +581,6 @@ void throttle_group_register_tgm(ThrottleGroupMember *tgm,
ThrottleState *ts = throttle_group_incref(groupname);
ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
- tgm->throttle_state = ts;
tgm->aio_context = ctx;
qatomic_set(&tgm->restart_pending, 0);
@@ -602,6 +601,9 @@ void throttle_group_register_tgm(ThrottleGroupMember *tgm,
read_timer_cb,
write_timer_cb,
tgm);
+
+ /* The I/O path reads this without tg->lock, so publish it last */
+ qatomic_store_release(&tgm->throttle_state, ts);
}
/* Unregister a ThrottleGroupMember from its group, removing it from the list,
--
2.53.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/1] block/throttle-groups: fix a NULL token crash on group creation
2026-09-08 14:55 [PATCH 0/1] block/throttle-groups: fix a NULL token crash on group creation Denis V. Lunev
2026-09-08 14:55 ` [PATCH] block/throttle-groups: fix crash when enabling I/O limits on a busy disk Denis V. Lunev
@ 2026-09-21 19:40 ` Denis V. Lunev
2026-09-22 14:47 ` Kevin Wolf
2 siblings, 0 replies; 5+ messages in thread
From: Denis V. Lunev @ 2026-09-21 19:40 UTC (permalink / raw)
To: Denis V. Lunev, qemu-devel
Cc: qemu-block, qemu-stable, Kevin Wolf, Hanna Reitz, Alberto Garcia
On 9/8/26 16:55, Denis V. Lunev wrote:
> This email originated from an IP that might not be authorized by the domain it was sent from.
> Do not click links or open attachments unless it is an email you expected to receive.
> Applying I/O limits to a disk that is already serving requests from an
> iothread can kill QEMU:
>
> #0 throttle_group_next_tgm (tgm=0x0) at block/throttle-groups.c:185
> #1 next_throttle_token (tgm=..., direction=THROTTLE_READ)
> #2 throttle_group_co_io_limits_intercept (bytes=8192, ...)
> #3 blk_co_do_preadv_part (blk=..., bytes=8192, ...)
> #4 blk_aio_read_entry (opaque=...)
> #5 coroutine_trampoline (i0=..., i1=...)
>
> throttle_group_register_tgm() publishes tgm->throttle_state before it
> takes tg->lock, and the group it points at is only filled in afterwards.
> A request that samples the pointer in between and wins tg->lock finds
> tg->tokens[] still NULL. The ordering is as old as the throttle group
> code, 76f4afb40f ("throttle: Add throttle group support").
>
> This was found from a production core. The group in it was brand new -
> name matching the disk, refcount 1, empty member list, both tokens NULL,
> a zeroed config because throttle_group_config() had not run yet, and
> tg->lock owned by the crashing iothread while the main thread sat
> blocked on the same lock inside throttle_group_register_tgm(). The
> trigger was libvirt applying blkdeviotune right after attaching a
> volume, while the guest was probing the new disk.
>
> The window is a couple of instructions wide. To see the bug on demand,
> widen it by hand:
>
> --- a/block/throttle-groups.c
> +++ b/block/throttle-groups.c
> @@ -584,6 +584,7 @@ void throttle_group_register_tgm(...)
> tgm->throttle_state = ts;
> tgm->aio_context = ctx;
> qatomic_set(&tgm->restart_pending, 0);
>
> + g_usleep(1000);
> QEMU_LOCK_GUARD(&tg->lock);
>
> With that in place, any I/O against the disk during a
> block_set_io_throttle that creates the group crashes at once, and the
> patch below makes it stop.
>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Hanna Reitz <hreitz@redhat.com>
> Cc: Alberto Garcia <berto@igalia.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
>
> Denis V. Lunev (1):
> block/throttle-groups: fix crash when enabling I/O limits on a busy
> disk
>
> block/block-backend.c | 12 ++++++------
> block/throttle-groups.c | 4 +++-
> 2 files changed, 9 insertions(+), 7 deletions(-)
>
ping
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 0/1] block/throttle-groups: fix a NULL token crash on group creation
2026-09-08 14:55 [PATCH 0/1] block/throttle-groups: fix a NULL token crash on group creation Denis V. Lunev
2026-09-08 14:55 ` [PATCH] block/throttle-groups: fix crash when enabling I/O limits on a busy disk Denis V. Lunev
2026-09-21 19:40 ` [PATCH 0/1] block/throttle-groups: fix a NULL token crash on group creation Denis V. Lunev
@ 2026-09-22 14:47 ` Kevin Wolf
2 siblings, 0 replies; 5+ messages in thread
From: Kevin Wolf @ 2026-09-22 14:47 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, qemu-block, qemu-stable, Hanna Reitz, Alberto Garcia
Am 08.09.2026 um 16:55 hat Denis V. Lunev geschrieben:
> Applying I/O limits to a disk that is already serving requests from an
> iothread can kill QEMU:
>
> #0 throttle_group_next_tgm (tgm=0x0) at block/throttle-groups.c:185
> #1 next_throttle_token (tgm=..., direction=THROTTLE_READ)
> #2 throttle_group_co_io_limits_intercept (bytes=8192, ...)
> #3 blk_co_do_preadv_part (blk=..., bytes=8192, ...)
> #4 blk_aio_read_entry (opaque=...)
> #5 coroutine_trampoline (i0=..., i1=...)
>
> throttle_group_register_tgm() publishes tgm->throttle_state before it
> takes tg->lock, and the group it points at is only filled in afterwards.
> A request that samples the pointer in between and wins tg->lock finds
> tg->tokens[] still NULL. The ordering is as old as the throttle group
> code, 76f4afb40f ("throttle: Add throttle group support").
>
> This was found from a production core. The group in it was brand new -
> name matching the disk, refcount 1, empty member list, both tokens NULL,
> a zeroed config because throttle_group_config() had not run yet, and
> tg->lock owned by the crashing iothread while the main thread sat
> blocked on the same lock inside throttle_group_register_tgm(). The
> trigger was libvirt applying blkdeviotune right after attaching a
> volume, while the guest was probing the new disk.
>
> The window is a couple of instructions wide. To see the bug on demand,
> widen it by hand:
>
> --- a/block/throttle-groups.c
> +++ b/block/throttle-groups.c
> @@ -584,6 +584,7 @@ void throttle_group_register_tgm(...)
> tgm->throttle_state = ts;
> tgm->aio_context = ctx;
> qatomic_set(&tgm->restart_pending, 0);
>
> + g_usleep(1000);
> QEMU_LOCK_GUARD(&tg->lock);
>
> With that in place, any I/O against the disk during a
> block_set_io_throttle that creates the group crashes at once, and the
> patch below makes it stop.
>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Hanna Reitz <hreitz@redhat.com>
> Cc: Alberto Garcia <berto@igalia.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
Thanks, applied to the block branch.
Kevin
^ permalink raw reply [flat|nested] 5+ messages in thread