Re: [Qemu-devel] [PATCH] linux-user: fix segmentation fault passing with g2h(x) != x

Re: [Qemu-devel] [PATCH] linux-user: fix segmentation fault passing with g2h(x) != x

[Alexander Graf]

On 25.06.2012, at 19:32, Alexander Graf wrote:

> When forwarding a segmentation fault into the guest process, we were passing
> the host's address directly into the guest process's signal descriptor.
> 
> That obviously confused the guest process, since it didn't know what to make
> of the (usually 32-bit truncated) address. Passing in g2h(address) makes the
> guest process a lot happier.
> 
> This fixes java running in arm-linux-user for me.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
> user-exec.c |   25 +++++++++++++------------
> 1 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/user-exec.c b/user-exec.c
> index 36d29b4..83d2d44 100644
> --- a/user-exec.c
> +++ b/user-exec.c
> @@ -100,19 +100,20 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned long address,
>     /* Maybe we're still holding the TB fiddling lock? */
>     spin_unlock_safe(&tb_lock);
> 
> -    /* XXX: locking issue */
> -    if (is_write && h2g_valid(address)
> -        && page_unprotect(h2g(address), pc, puc)) {
> -        return 1;
> -    }
> +    if (h2g_valid(address)) {

This is broken. The address can be outside of RESERVED_VA, but still inside of the guest virtual address space, thus a valid segv.


Alex

> +        /* XXX: locking issue */
> +        if (is_write && page_unprotect(h2g(address), pc, puc)) {
> +            return 1;
> +        }
> 
> -    /* see if it is an MMU fault */
> -    ret = cpu_handle_mmu_fault(env, address, is_write, MMU_USER_IDX);
> -    if (ret < 0) {
> -        return 0; /* not an MMU fault */
> -    }
> -    if (ret == 0) {
> -        return 1; /* the MMU fault was handled without causing real CPU fault */
> +        /* see if it is an MMU fault */
> +        ret = cpu_handle_mmu_fault(env, h2g(address), is_write, MMU_USER_IDX);
> +        if (ret < 0) {
> +            return 0; /* not an MMU fault */
> +        }
> +        if (ret == 0) {
> +            return 1; /* the MMU fault was handled without causing real CPU fault */
> +        }
>     }
>     /* now we have a real cpu fault */
>     tb = tb_find_pc(pc);
> -- 
> 1.6.0.2
> 
>