diff options
author | Pierrick Bouvier <pierrick.bouvier@linaro.org> | 2025-07-24 09:11:42 -0700 |
---|---|---|
committer | Philippe Mathieu-Daudé <philmd@linaro.org> | 2025-07-29 13:56:39 +0200 |
commit | 2865bf1c5795371ef441e9029bc22566ccff8299 (patch) | |
tree | 5239022485b8dd4d44e33ea120f1a60ec30eb5c1 /hw/avr/arduino.c | |
parent | 014bb30d218ef0d91c300324dec99138e999e32d (diff) | |
download | qemu-2865bf1c5795371ef441e9029bc22566ccff8299.zip qemu-2865bf1c5795371ef441e9029bc22566ccff8299.tar.gz qemu-2865bf1c5795371ef441e9029bc22566ccff8299.tar.bz2 |
system/physmem: fix use-after-free with dispatch
A use-after-free bug was reported when booting a Linux kernel during the
pci setup phase. It's quite hard to reproduce (needs smp, and favored by
having several pci devices with BAR and specific Linux config, which
is Debian default one in this case).
After investigation (see the associated bug ticket), it appears that,
under specific conditions, we might access a cached AddressSpaceDispatch
that was reclaimed by RCU thread meanwhile.
In the Linux boot scenario, during the pci phase, memory region are
destroyed/recreated, resulting in exposition of the bug.
The core of the issue is that we cache the dispatch associated to
current cpu in cpu->cpu_ases[asidx].memory_dispatch. It is updated with
tcg_commit, which runs asynchronously on a given cpu.
At some point, we leave the rcu critial section, and the RCU thread
starts reclaiming it, but tcg_commit is not yet invoked, resulting in
the use-after-free.
It's not the first problem around this area, and commit 0d58c660689 [1]
("softmmu: Use async_run_on_cpu in tcg_commit") already tried to
address it. It did a good job, but it seems that we found a specific
situation where it's not enough.
This patch takes a simple approach: remove the cached value creating the
issue, and make sure we always get the current mapping for address
space, using address_space_to_dispatch(cpu->cpu_ases[asidx].as).
It's equivalent to qatomic_rcu_read(&as->current_map)->dispatch;
This is not really costly, we just need two dereferences,
including one atomic (rcu) read, which is negligible considering we are
already on mmu slow path anyway.
Note that tcg_commit is still needed, as it's taking care of flushing
TLB, removing previously mapped entries.
Another solution would be to cache directly values under the dispatch
(dispatch themselves are not ref counted), keep an active reference on
associated memory section, and release it when appropriate (tricky).
Given the time already spent debugging this area now and previously, I
strongly prefer eliminating the root of the issue, instead of adding
more complexity for a hypothetical performance gain. RCU is precisely
used to ensure good performance when reading data, so caching is not as
beneficial as it might seem IMHO.
[1] https://gitlab.com/qemu-project/qemu/-/commit/0d58c660689f6da1e3feff8a997014003d928b3b
Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3040
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>
Tested-by: Michael Tokarev <mjt@tls.msk.ru>
Message-ID: <20250724161142.2803091-1-pierrick.bouvier@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Diffstat (limited to 'hw/avr/arduino.c')
0 files changed, 0 insertions, 0 deletions