[PATCH] target/ppc: Stop vCPU thread before calling parent_unrealize

Shivang Upadhyay posted 1 patch 3 days, 7 hours ago
target/ppc/cpu_init.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] target/ppc: Stop vCPU thread before calling parent_unrealize
Posted by Shivang Upadhyay 3 days, 7 hours ago
During CPU hot-unplug (e.g. via dynamic reconfiguration unplug),
ppc_cpu_unrealize() invoked pcc->parent_unrealize(dev) before calling
cpu_remove_sync(CPU(cpu)).

pcc->parent_unrealize() calls cpu_common_unrealize(), which triggers
accel_cpu_common_unrealize() -> tcg_exec_unrealizefn() -> tlb_destroy().
This immediately frees the CPU's TLB tables and structures. Because the
vCPU thread had not yet been stopped and joined via cpu_remove_sync(),
the vCPU thread was still actively running its event loop and processing
queued CPU work (such as tcg_commit_cpu / tlb_flush).

This resulted in a race where the running vCPU thread accessed and freed
already-destroyed TLB tables concurrently with tlb_destroy(), leading to
Segfault (due to heap corruption).

AddressSanitizer build reported a double-free:

=================================================================
==121930==ERROR: AddressSanitizer: attempting double-free on 0x7ef8f3438800 in thread T14:
    #0 0x7fe8f74e5beb in free.part.0 (/lib64/libasan.so.8+0xe5beb)
    #1 0x7fe8f6cb8f84 in g_free (/lib64/libglib-2.0.so.0+0x41f84)
    #2 0x558bf6a391b1 in tlb_mmu_resize_locked accel/tcg/cputlb.c:249
    #3 0x558bf6a396b5 in tlb_flush_one_mmuidx_locked accel/tcg/cputlb.c:296
    #4 0x558bf6a39f91 in tlb_flush_by_mmuidx_async_work accel/tcg/cputlb.c:390
    #5 0x558bf6a3a200 in tlb_flush_by_mmuidx accel/tcg/cputlb.c:417
    #6 0x558bf6a3a22a in tlb_flush accel/tcg/cputlb.c:422
    #7 0x558bf73f31ac in tcg_commit_cpu system/physmem.c:3068
    #8 0x558bf6987c55 in process_queued_cpu_work cpu-common.c:378
    #9 0x558bf73a9913 in qemu_process_cpu_events_common system/cpus.c:402
    #10 0x558bf73a9a46 in qemu_process_cpu_events system/cpus.c:421
    #11 0x558bf6a65974 in mttcg_cpu_thread_fn accel/tcg/tcg-accel-ops-mttcg.c:90

0x7ef8f3438800 is located 0 bytes inside of 65536-byte region [0x7ef8f3438800,0x7ef8f3448800)
freed by thread T9 here:
    #0 0x7fe8f74e5beb in free.part.0 (/lib64/libasan.so.8+0xe5beb)
    #1 0x7fe8f6cb8f84 in g_free (/lib64/libglib-2.0.so.0+0x41f84)
    #2 0x558bf6a39a91 in tlb_destroy accel/tcg/cputlb.c:345
    #3 0x558bf6a16354 in tcg_exec_unrealizefn accel/tcg/cpu-exec.c:1094
    #4 0x558bf693d073 in accel_cpu_common_unrealize accel/accel-common.c:117
    #5 0x558bf6980e37 in cpu_common_unrealize hw/core/cpu-common.c:279
    #6 0x558bf6980dfa in cpu_common_unrealizefn hw/core/cpu-common.c:267
    #7 0x558bf763ef65 in ppc_cpu_unrealize target/ppc/cpu_init.c:6965
    #8 0x558bf7872199 in device_set_realized hw/core/qdev.c:618
    #14 0x558bf756068f in spapr_unrealize_vcpu hw/ppc/spapr_cpu_core.c:209

Fix this by moving cpu_remove_sync() before pcc->parent_unrealize(dev)
in ppc_cpu_unrealize(), ensuring the vCPU thread is stopped, has
finished processing its events, and is joined before CPU resources
and accelerator state are destroyed.

Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
---
 target/ppc/cpu_init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 6c626843c9..b711f9c0a8 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6962,10 +6962,10 @@ static void ppc_cpu_unrealize(DeviceState *dev)
     PowerPCCPU *cpu = POWERPC_CPU(dev);
     PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
 
-    pcc->parent_unrealize(dev);
-
     cpu_remove_sync(CPU(cpu));
 
+    pcc->parent_unrealize(dev);
+
     destroy_ppc_opcodes(cpu);
 }
 
-- 
2.54.0
Re: [PATCH] target/ppc: Stop vCPU thread before calling parent_unrealize
Posted by Amit Machhiwal 2 days, 4 hours ago
On 2026/09/23 05:44 PM, Shivang Upadhyay wrote:
> During CPU hot-unplug (e.g. via dynamic reconfiguration unplug),
> ppc_cpu_unrealize() invoked pcc->parent_unrealize(dev) before calling
> cpu_remove_sync(CPU(cpu)).
> 
> pcc->parent_unrealize() calls cpu_common_unrealize(), which triggers
> accel_cpu_common_unrealize() -> tcg_exec_unrealizefn() -> tlb_destroy().
> This immediately frees the CPU's TLB tables and structures. Because the
> vCPU thread had not yet been stopped and joined via cpu_remove_sync(),
> the vCPU thread was still actively running its event loop and processing
> queued CPU work (such as tcg_commit_cpu / tlb_flush).
> 
> This resulted in a race where the running vCPU thread accessed and freed
> already-destroyed TLB tables concurrently with tlb_destroy(), leading to
> Segfault (due to heap corruption).
> 
> AddressSanitizer build reported a double-free:
> 
> =================================================================
> ==121930==ERROR: AddressSanitizer: attempting double-free on 0x7ef8f3438800 in thread T14:
>     #0 0x7fe8f74e5beb in free.part.0 (/lib64/libasan.so.8+0xe5beb)
>     #1 0x7fe8f6cb8f84 in g_free (/lib64/libglib-2.0.so.0+0x41f84)
>     #2 0x558bf6a391b1 in tlb_mmu_resize_locked accel/tcg/cputlb.c:249
>     #3 0x558bf6a396b5 in tlb_flush_one_mmuidx_locked accel/tcg/cputlb.c:296
>     #4 0x558bf6a39f91 in tlb_flush_by_mmuidx_async_work accel/tcg/cputlb.c:390
>     #5 0x558bf6a3a200 in tlb_flush_by_mmuidx accel/tcg/cputlb.c:417
>     #6 0x558bf6a3a22a in tlb_flush accel/tcg/cputlb.c:422
>     #7 0x558bf73f31ac in tcg_commit_cpu system/physmem.c:3068
>     #8 0x558bf6987c55 in process_queued_cpu_work cpu-common.c:378
>     #9 0x558bf73a9913 in qemu_process_cpu_events_common system/cpus.c:402
>     #10 0x558bf73a9a46 in qemu_process_cpu_events system/cpus.c:421
>     #11 0x558bf6a65974 in mttcg_cpu_thread_fn accel/tcg/tcg-accel-ops-mttcg.c:90
> 
> 0x7ef8f3438800 is located 0 bytes inside of 65536-byte region [0x7ef8f3438800,0x7ef8f3448800)
> freed by thread T9 here:
>     #0 0x7fe8f74e5beb in free.part.0 (/lib64/libasan.so.8+0xe5beb)
>     #1 0x7fe8f6cb8f84 in g_free (/lib64/libglib-2.0.so.0+0x41f84)
>     #2 0x558bf6a39a91 in tlb_destroy accel/tcg/cputlb.c:345
>     #3 0x558bf6a16354 in tcg_exec_unrealizefn accel/tcg/cpu-exec.c:1094
>     #4 0x558bf693d073 in accel_cpu_common_unrealize accel/accel-common.c:117
>     #5 0x558bf6980e37 in cpu_common_unrealize hw/core/cpu-common.c:279
>     #6 0x558bf6980dfa in cpu_common_unrealizefn hw/core/cpu-common.c:267
>     #7 0x558bf763ef65 in ppc_cpu_unrealize target/ppc/cpu_init.c:6965
>     #8 0x558bf7872199 in device_set_realized hw/core/qdev.c:618
>     #14 0x558bf756068f in spapr_unrealize_vcpu hw/ppc/spapr_cpu_core.c:209
> 
> Fix this by moving cpu_remove_sync() before pcc->parent_unrealize(dev)
> in ppc_cpu_unrealize(), ensuring the vCPU thread is stopped, has
> finished processing its events, and is joined before CPU resources
> and accelerator state are destroyed.
> 
> Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
> ---
>  target/ppc/cpu_init.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 6c626843c9..b711f9c0a8 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -6962,10 +6962,10 @@ static void ppc_cpu_unrealize(DeviceState *dev)
>      PowerPCCPU *cpu = POWERPC_CPU(dev);
>      PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
>  
> -    pcc->parent_unrealize(dev);
> -
>      cpu_remove_sync(CPU(cpu));
>  
> +    pcc->parent_unrealize(dev);
> +

The fix is correct and the commit message is well written.  The ASan trace makes
the race unambiguous: tlb_destroy() frees fast->table and desc->fulltlb while
the MTTCG thread is still running tlb_mmu_resize_locked() on the same structures
via the queued tcg_commit_cpu work.

Reviewed-by: Amit Machhiwal <amachhiw@linux.ibm.com>

Minor: I think a short comment above cpu_remove_sync() would make the ordering
constraint self-documenting, e.g.:

    /*
     * Stop and join the vCPU thread before tearing down accelerator
     * state (TLB tables etc.) to prevent the running thread from
     * accessing freed resources.
     */
    cpu_remove_sync(CPU(cpu));

Thanks,
Amit
Re: [PATCH] target/ppc: Stop vCPU thread before calling parent_unrealize
Posted by Shivang Upadhyay 1 day, 11 hours ago
On Thu, 2026-09-24 at 20:12 +0530, Amit Machhiwal wrote:
> The fix is correct and the commit message is well written.  The ASan
> trace makes
> the race unambiguous: tlb_destroy() frees fast->table and desc-
> >fulltlb while
> the MTTCG thread is still running tlb_mmu_resize_locked() on the same
> structures
> via the queued tcg_commit_cpu work.
> 
> Reviewed-by: Amit Machhiwal <amachhiw@linux.ibm.com>
> 
Thanks for review.

> Minor: I think a short comment above cpu_remove_sync() would make the
> ordering
> constraint self-documenting, e.g.:
> 
>     /*
>      * Stop and join the vCPU thread before tearing down accelerator
>      * state (TLB tables etc.) to prevent the running thread from
>      * accessing freed resources.
>      */
>     cpu_remove_sync(CPU(cpu));

Seems this is already pulled via misc tree.

I plan to work on Philippe's suggestion to pull all cpu_remove_sync()
calls into cpu_common_unrealize(), that should take care of all such
caveats.

~Shivang.
Re: [PATCH] target/ppc: Stop vCPU thread before calling parent_unrealize
Posted by Mukesh Kumar Chaurasiya 2 days, 10 hours ago
On Wed, Sep 23, 2026 at 05:44:44PM +0530, Shivang Upadhyay wrote:
> During CPU hot-unplug (e.g. via dynamic reconfiguration unplug),
> ppc_cpu_unrealize() invoked pcc->parent_unrealize(dev) before calling
> cpu_remove_sync(CPU(cpu)).
> 
> pcc->parent_unrealize() calls cpu_common_unrealize(), which triggers
> accel_cpu_common_unrealize() -> tcg_exec_unrealizefn() -> tlb_destroy().
> This immediately frees the CPU's TLB tables and structures. Because the
> vCPU thread had not yet been stopped and joined via cpu_remove_sync(),
> the vCPU thread was still actively running its event loop and processing
> queued CPU work (such as tcg_commit_cpu / tlb_flush).
> 
> This resulted in a race where the running vCPU thread accessed and freed
> already-destroyed TLB tables concurrently with tlb_destroy(), leading to
> Segfault (due to heap corruption).
> 
> AddressSanitizer build reported a double-free:
> 
> =================================================================
> ==121930==ERROR: AddressSanitizer: attempting double-free on 0x7ef8f3438800 in thread T14:
>     #0 0x7fe8f74e5beb in free.part.0 (/lib64/libasan.so.8+0xe5beb)
>     #1 0x7fe8f6cb8f84 in g_free (/lib64/libglib-2.0.so.0+0x41f84)
>     #2 0x558bf6a391b1 in tlb_mmu_resize_locked accel/tcg/cputlb.c:249
>     #3 0x558bf6a396b5 in tlb_flush_one_mmuidx_locked accel/tcg/cputlb.c:296
>     #4 0x558bf6a39f91 in tlb_flush_by_mmuidx_async_work accel/tcg/cputlb.c:390
>     #5 0x558bf6a3a200 in tlb_flush_by_mmuidx accel/tcg/cputlb.c:417
>     #6 0x558bf6a3a22a in tlb_flush accel/tcg/cputlb.c:422
>     #7 0x558bf73f31ac in tcg_commit_cpu system/physmem.c:3068
>     #8 0x558bf6987c55 in process_queued_cpu_work cpu-common.c:378
>     #9 0x558bf73a9913 in qemu_process_cpu_events_common system/cpus.c:402
>     #10 0x558bf73a9a46 in qemu_process_cpu_events system/cpus.c:421
>     #11 0x558bf6a65974 in mttcg_cpu_thread_fn accel/tcg/tcg-accel-ops-mttcg.c:90
> 
> 0x7ef8f3438800 is located 0 bytes inside of 65536-byte region [0x7ef8f3438800,0x7ef8f3448800)
> freed by thread T9 here:
>     #0 0x7fe8f74e5beb in free.part.0 (/lib64/libasan.so.8+0xe5beb)
>     #1 0x7fe8f6cb8f84 in g_free (/lib64/libglib-2.0.so.0+0x41f84)
>     #2 0x558bf6a39a91 in tlb_destroy accel/tcg/cputlb.c:345
>     #3 0x558bf6a16354 in tcg_exec_unrealizefn accel/tcg/cpu-exec.c:1094
>     #4 0x558bf693d073 in accel_cpu_common_unrealize accel/accel-common.c:117
>     #5 0x558bf6980e37 in cpu_common_unrealize hw/core/cpu-common.c:279
>     #6 0x558bf6980dfa in cpu_common_unrealizefn hw/core/cpu-common.c:267
>     #7 0x558bf763ef65 in ppc_cpu_unrealize target/ppc/cpu_init.c:6965
>     #8 0x558bf7872199 in device_set_realized hw/core/qdev.c:618
>     #14 0x558bf756068f in spapr_unrealize_vcpu hw/ppc/spapr_cpu_core.c:209
> 
> Fix this by moving cpu_remove_sync() before pcc->parent_unrealize(dev)
> in ppc_cpu_unrealize(), ensuring the vCPU thread is stopped, has
> finished processing its events, and is joined before CPU resources
> and accelerator state are destroyed.
> 
> Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
> ---
>  target/ppc/cpu_init.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 6c626843c9..b711f9c0a8 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -6962,10 +6962,10 @@ static void ppc_cpu_unrealize(DeviceState *dev)
>      PowerPCCPU *cpu = POWERPC_CPU(dev);
>      PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
>  
> -    pcc->parent_unrealize(dev);
> -
>      cpu_remove_sync(CPU(cpu));
>  
> +    pcc->parent_unrealize(dev);
> +
>      destroy_ppc_opcodes(cpu);
>  }
>  
> -- 
> 2.54.0
> 
Makes Sense.

Reviewed-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com>
>
Re: [PATCH] target/ppc: Stop vCPU thread before calling parent_unrealize
Posted by Shivang Upadhyay 2 days, 5 hours ago
On Thu, 2026-09-24 at 14:42 +0530, Mukesh Kumar Chaurasiya wrote:
> Makes Sense.
> 
> Reviewed-by: Mukesh Kumar Chaurasiya (IBM) <mkchauras@gmail.com>
Thanks for your review Mukesh :)

~Shivang.
Re: [PATCH] target/ppc: Stop vCPU thread before calling parent_unrealize
Posted by Philippe Mathieu-Daudé 3 days, 5 hours ago
Hi,

On 23/9/26 14:14, Shivang Upadhyay wrote:
> During CPU hot-unplug (e.g. via dynamic reconfiguration unplug),
> ppc_cpu_unrealize() invoked pcc->parent_unrealize(dev) before calling
> cpu_remove_sync(CPU(cpu)).
> 
> pcc->parent_unrealize() calls cpu_common_unrealize(), which triggers
> accel_cpu_common_unrealize() -> tcg_exec_unrealizefn() -> tlb_destroy().
> This immediately frees the CPU's TLB tables and structures. Because the
> vCPU thread had not yet been stopped and joined via cpu_remove_sync(),
> the vCPU thread was still actively running its event loop and processing
> queued CPU work (such as tcg_commit_cpu / tlb_flush).
> 
> This resulted in a race where the running vCPU thread accessed and freed
> already-destroyed TLB tables concurrently with tlb_destroy(), leading to
> Segfault (due to heap corruption).
> 
> AddressSanitizer build reported a double-free:
> 
> =================================================================
> ==121930==ERROR: AddressSanitizer: attempting double-free on 0x7ef8f3438800 in thread T14:
>      #0 0x7fe8f74e5beb in free.part.0 (/lib64/libasan.so.8+0xe5beb)
>      #1 0x7fe8f6cb8f84 in g_free (/lib64/libglib-2.0.so.0+0x41f84)
>      #2 0x558bf6a391b1 in tlb_mmu_resize_locked accel/tcg/cputlb.c:249
>      #3 0x558bf6a396b5 in tlb_flush_one_mmuidx_locked accel/tcg/cputlb.c:296
>      #4 0x558bf6a39f91 in tlb_flush_by_mmuidx_async_work accel/tcg/cputlb.c:390
>      #5 0x558bf6a3a200 in tlb_flush_by_mmuidx accel/tcg/cputlb.c:417
>      #6 0x558bf6a3a22a in tlb_flush accel/tcg/cputlb.c:422
>      #7 0x558bf73f31ac in tcg_commit_cpu system/physmem.c:3068
>      #8 0x558bf6987c55 in process_queued_cpu_work cpu-common.c:378
>      #9 0x558bf73a9913 in qemu_process_cpu_events_common system/cpus.c:402
>      #10 0x558bf73a9a46 in qemu_process_cpu_events system/cpus.c:421
>      #11 0x558bf6a65974 in mttcg_cpu_thread_fn accel/tcg/tcg-accel-ops-mttcg.c:90
> 
> 0x7ef8f3438800 is located 0 bytes inside of 65536-byte region [0x7ef8f3438800,0x7ef8f3448800)
> freed by thread T9 here:
>      #0 0x7fe8f74e5beb in free.part.0 (/lib64/libasan.so.8+0xe5beb)
>      #1 0x7fe8f6cb8f84 in g_free (/lib64/libglib-2.0.so.0+0x41f84)
>      #2 0x558bf6a39a91 in tlb_destroy accel/tcg/cputlb.c:345
>      #3 0x558bf6a16354 in tcg_exec_unrealizefn accel/tcg/cpu-exec.c:1094
>      #4 0x558bf693d073 in accel_cpu_common_unrealize accel/accel-common.c:117
>      #5 0x558bf6980e37 in cpu_common_unrealize hw/core/cpu-common.c:279
>      #6 0x558bf6980dfa in cpu_common_unrealizefn hw/core/cpu-common.c:267
>      #7 0x558bf763ef65 in ppc_cpu_unrealize target/ppc/cpu_init.c:6965
>      #8 0x558bf7872199 in device_set_realized hw/core/qdev.c:618
>      #14 0x558bf756068f in spapr_unrealize_vcpu hw/ppc/spapr_cpu_core.c:209
> 
> Fix this by moving cpu_remove_sync() before pcc->parent_unrealize(dev)
> in ppc_cpu_unrealize(), ensuring the vCPU thread is stopped, has
> finished processing its events, and is joined before CPU resources
> and accelerator state are destroyed.
> 
> Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
> ---
>   target/ppc/cpu_init.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Good enough for now:

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

But long term we should move the call to cpu_common_unrealize() IMO,
it shouldn't be called per-target.

Re: [PATCH] target/ppc: Stop vCPU thread before calling parent_unrealize
Posted by Shivang Upadhyay 2 days, 5 hours ago
On Wed, 2026-09-23 at 15:39 +0200, Philippe Mathieu-Daudé wrote:
> > Signed-off-by: Shivang Upadhyay <shivangu@linux.ibm.com>
> > ---
> >    target/ppc/cpu_init.c | 4 ++--
> >    1 file changed, 2 insertions(+), 2 deletions(-)
> 
> Good enough for now:
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> 
Thanks Philippe.

> But long term we should move the call to cpu_common_unrealize() IMO,
> it shouldn't be called per-target.

Good suggestion, As this should be same for all archs.

~Shivang.