Re: [PATCH 1/2] target/s390x: Fix MVC not always invalidating translation blocks

[Richard Henderson]

On 1/27/25 16:12, Ilya Leoshkevich wrote:
> Node.js crashes in qemu-system-s390x with random SIGSEGVs / SIGILLs.
>
> The v8 JIT used by Node.js can garbage collect and overwrite unused
> code. Overwriting is performed by WritableJitAllocation::CopyCode(),
> which ultimately calls memcpy(). For certain sizes, memcpy() uses the
> MVC instruction.
>
> QEMU implements MVC and other similar instructions using helpers. While
> TCG store ops invalidate affected translation blocks automatically,
> helpers must do this manually by calling probe_access_flags(). The MVC
> helper does this using the access_prepare() -> access_prepare_nf() ->
> s390_probe_access() -> probe_access_flags() call chain.
>
> At the last step of this chain, the store size is replaced with 0. This
> causes the probe_access_flags() -> notdirty_write() ->
> tb_invalidate_phys_range_fast() chain to miss some translation blocks.
>
> When this happens, QEMU executes a mix of old and new code. This
> quickly leads to either a SIGSEGV or a SIGILL in case the old code
> ends in the middle of a new instruction.
>
> Fix by passing the true size.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

> Reported-by: Berthold Gunreben <azouhr@opensuse.org>
> Cc: Sarah Kriesch <ada.lovelace@gmx.de>
> Cc: qemu-stable@nongnu.org
> Closes: https://bugzilla.opensuse.org/show_bug.cgi?id=1235709
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   target/s390x/tcg/mem_helper.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
> index 32717acb7d1..c6ab2901e5a 100644
> --- a/target/s390x/tcg/mem_helper.c
> +++ b/target/s390x/tcg/mem_helper.c
> @@ -149,7 +149,7 @@ static inline int s390_probe_access(CPUArchState *env, 
> target_ulong addr,
>                                       int mmu_idx, bool nonfault,
>                                       void **phost, uintptr_t ra)
>   {
> -    int flags = probe_access_flags(env, addr, 0, access_type, mmu_idx,
> +    int flags = probe_access_flags(env, addr, size, access_type, mmu_idx,
>                                      nonfault, phost, ra);
>   
>       if (unlikely(flags & TLB_INVALID_MASK)) {