From: "Denis V. Lunev" <[email protected]> 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. The result is usual use-after-free and qemu crash.
Take stats->lock in both setters, matching the lock already held by the reader. 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]> Message-ID: <[email protected]> Reviewed-by: Kevin Wolf <[email protected]> Signed-off-by: Kevin Wolf <[email protected]> (cherry picked from commit 5b0ba385a024ad92c63c175317cd9374de2e6165) Signed-off-by: Michael Tokarev <[email protected]> diff --git a/block/accounting.c b/block/accounting.c index c7feca40ab1..c5584abb76e 100644 --- a/block/accounting.c +++ b/block/accounting.c @@ -182,6 +182,8 @@ int block_latency_histogram_set(BlockAcctStats *stats, enum BlockAcctType type, return -EINVAL; } + qemu_mutex_lock(&stats->lock); + hist->nbins = new_nbins; g_free(hist->boundaries); hist->boundaries = g_new(uint64_t, hist->nbins - 1); @@ -194,6 +196,8 @@ int block_latency_histogram_set(BlockAcctStats *stats, enum BlockAcctType type, g_free(hist->bins); hist->bins = g_new0(uint64_t, hist->nbins); + qemu_mutex_unlock(&stats->lock); + return 0; } @@ -201,12 +205,16 @@ void block_latency_histograms_clear(BlockAcctStats *stats) { int i; + qemu_mutex_lock(&stats->lock); + for (i = 0; i < BLOCK_MAX_IOTYPE; i++) { BlockLatencyHistogram *hist = &stats->latency_histogram[i]; g_free(hist->bins); g_free(hist->boundaries); memset(hist, 0, sizeof(*hist)); } + + qemu_mutex_unlock(&stats->lock); } static void block_account_one_io(BlockAcctStats *stats, BlockAcctCookie *cookie, -- 2.47.3
