aboutsummaryrefslogtreecommitdiff
path: root/hw/avr/arduino.c
diff options
context:
space:
mode:
authorPierrick Bouvier <pierrick.bouvier@linaro.org>2025-07-24 09:11:42 -0700
committerPhilippe Mathieu-Daudé <philmd@linaro.org>2025-07-29 13:56:39 +0200
commit2865bf1c5795371ef441e9029bc22566ccff8299 (patch)
tree5239022485b8dd4d44e33ea120f1a60ec30eb5c1 /hw/avr/arduino.c
parent014bb30d218ef0d91c300324dec99138e999e32d (diff)
downloadqemu-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