block_latency_histogram_set() and block_latency_histograms_clear()
replace BlockLatencyHistogram's nbins/boundaries/bins without taking
stats->lock, while block_account_one_io() reads those same fields
under that lock from whatever iothread completes the I/O. A histogram
reconfiguration (block-latency-histogram-set QMP command, monitor
thread) racing an in-flight completion can therefore observe those
fields torn, hitting assert(pos != NULL) in
block_latency_histogram_account(), or corrupting the heap outright.
This showed up as a qemu-kvm SIGABRT on a customer's virtio-blk guest.

Regression test is added for illustrative purpose but I am unsure that
it is viable long term. Feel free to drop.

v1 -> v2
* patch 1, 2: WITH_QEMU_LOCK_GUARD -> plain lock()/unlock(), pure
  additions, nothing reindented or moved
* patch 3: writer thread now also calls
  block_latency_histograms_clear(), the other function patch 1 fixes
* patch 2: take stats->lock for bdrv_query_blk_stats()'s whole call
  instead of per-section, since the interval loop's
  timed_average_min/max/avg() calls race timed_average_account() the
  same way the already-locked fields did; block_acct_queue_depth()
  now requires the caller to hold the lock instead of taking it
  itself, since bdrv_query_blk_stats() is its only caller

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>

Denis V. Lunev (3):
  block/accounting: take stats->lock in latency histogram setters
  block/qapi: take stats->lock when reading BlockAcctStats for
    query-blockstats
  tests/unit: add reproducer for BlockAcctStats histogram locking race

 block/accounting.c                 |  11 ++-
 block/qapi.c                       |   3 +
 include/block/accounting.h         |   1 +
 tests/unit/meson.build             |   1 +
 tests/unit/test-block-accounting.c | 115 +++++++++++++++++++++++++++++
 5 files changed, 129 insertions(+), 2 deletions(-)
 create mode 100644 tests/unit/test-block-accounting.c

-- 
2.53.0

Am 24.07.2026 um 13:13 hat Denis V. Lunev geschrieben:
> block_latency_histogram_set() and block_latency_histograms_clear()
> replace BlockLatencyHistogram's nbins/boundaries/bins without taking
> stats->lock, while block_account_one_io() reads those same fields
> under that lock from whatever iothread completes the I/O. A histogram
> reconfiguration (block-latency-histogram-set QMP command, monitor
> thread) racing an in-flight completion can therefore observe those
> fields torn, hitting assert(pos != NULL) in
> block_latency_histogram_account(), or corrupting the heap outright.
> This showed up as a qemu-kvm SIGABRT on a customer's virtio-blk guest.
> 
> Regression test is added for illustrative purpose but I am unsure that
> it is viable long term. Feel free to drop.
> 
> v1 -> v2
> * patch 1, 2: WITH_QEMU_LOCK_GUARD -> plain lock()/unlock(), pure
>   additions, nothing reindented or moved
> * patch 3: writer thread now also calls
>   block_latency_histograms_clear(), the other function patch 1 fixes
> * patch 2: take stats->lock for bdrv_query_blk_stats()'s whole call
>   instead of per-section, since the interval loop's
>   timed_average_min/max/avg() calls race timed_average_account() the
>   same way the already-locked fields did; block_acct_queue_depth()
>   now requires the caller to hold the lock instead of taking it
>   itself, since bdrv_query_blk_stats() is its only caller

Thanks, applied to the block branch.

Kevin