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 <[email protected]> CC: Kevin Wolf <[email protected]> CC: Hanna Reitz <[email protected]> CC: Vladimir Sementsov-Ogievskiy <[email protected]> CC: Andrey Drobyshev <[email protected]> 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
