[PULL,43/47] target/arm: fix AArch64 virtual address space size

From: Remi Denis-Courmont <remi.denis.courmont@huawei.com>

Since QEMU does not support the ARMv8.2-LVA, Large Virtual Address,
extension (yet), the VA address space is 48-bits plus a sign bit. User
mode can only handle the positive half of the address space, so that
makes a limit of 48 bits.

(With LVA, it would be 53 and 52 bits respectively.)

The incorrectly large address space conflicts with PAuth instructions,
which use bits 48-54 and 56-63 for the pointer authentication code. This
also conflicts with (as yet unsupported by QEMU) data tagging and with
the ARMv8.5-MTE extension.

Signed-off-by: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On 01/02/2019 17:06, Peter Maydell wrote:
> From: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
> 
> Since QEMU does not support the ARMv8.2-LVA, Large Virtual Address,
> extension (yet), the VA address space is 48-bits plus a sign bit. User
> mode can only handle the positive half of the address space, so that
> makes a limit of 48 bits.
> 
> (With LVA, it would be 53 and 52 bits respectively.)
> 
> The incorrectly large address space conflicts with PAuth instructions,
> which use bits 48-54 and 56-63 for the pointer authentication code. This
> also conflicts with (as yet unsupported by QEMU) data tagging and with
> the ARMv8.5-MTE extension.
> 
> Signed-off-by: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/cpu.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 63934a200ad..a68bcc9fedb 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -2512,7 +2512,7 @@ bool write_cpustate_to_list(ARMCPU *cpu);
>  
>  #if defined(TARGET_AARCH64)
>  #  define TARGET_PHYS_ADDR_SPACE_BITS 48
> -#  define TARGET_VIRT_ADDR_SPACE_BITS 64
> +#  define TARGET_VIRT_ADDR_SPACE_BITS 48
>  #else
>  #  define TARGET_PHYS_ADDR_SPACE_BITS 40
>  #  define TARGET_VIRT_ADDR_SPACE_BITS 32
> 

This change breaks qemu-aarch64 (using LTP test suite):

# chroot chroot/arm64/bionic /opt/ltp/testcases/bin/access03
tst_test.c:1015: INFO: Timeout per run is 0h 05m 00s
qemu-aarch64: .../qemu/accel/tcg/translate-all.c:2522: page_check_range: Assertion `start < ((target_ulong)1 << L1_MAP_ADDR_SPACE_BITS)' failed.
qemu:handle_cpu_signal received signal outside vCPU context @ pc=0x60001554

Any idea?
Thanks,
Laurent

Hi,

That LTP test case deliberately tries to invoke a system call with an invalid address to make sure that the kernel fails safely. There seems to be an impedance mismatch between access_ok() and page_check_range() here, where the later assumes that the address is in guest range:

    /* This function should never be called with addresses outside the
       guest address space.  If this assert fires, it probably indicates
       a missing call to h2g_valid.  */

It looks to me that there actually two separate bugs here:

1) access_ok() does not fulfill the requirements of page_check_range(), misses a guest_addr_valid() call.
2) The page_check_range() assertion does not account for potential overflow, although the non-debug code does account for it.

On 08/02/2019 16:38, Remi Denis Courmont wrote:
>    Hi,
> 
> That LTP test case deliberately tries to invoke a system call with an invalid address to make sure that the kernel fails safely. There seems to be an impedance mismatch between access_ok() and page_check_range() here, where the later assumes that the address is in guest range:
> 
>     /* This function should never be called with addresses outside the
>        guest address space.  If this assert fires, it probably indicates
>        a missing call to h2g_valid.  */
> 
> It looks to me that there actually two separate bugs here:
> 
> 1) access_ok() does not fulfill the requirements of page_check_range(), misses a guest_addr_valid() call.
> 2) The page_check_range() assertion does not account for potential overflow, although the non-debug code does account for it.

You're right, this fixes the problem for me:

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index ef400cb78a..a8dce7f821 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -457,6 +457,9 @@ extern unsigned long guest_stack_size;
 
 static inline int access_ok(int type, abi_ulong addr, abi_ulong size)
 {
+    if (!h2g_valid(addr)) {
+        return 0;
+    }
     return page_check_range((target_ulong)addr, size,
                             (type == VERIFY_READ) ? PAGE_READ : (PAGE_READ | PAGE_WRITE)) == 0;
 }

> ________________________________________
> De : Laurent Vivier [lvivier@redhat.com]
> Envoyé : vendredi 8 février 2019 16:02
> À : Peter Maydell; qemu-devel@nongnu.org; Remi Denis Courmont; Richard Henderson
> Objet : Re: [Qemu-devel] [PULL 43/47] target/arm: fix AArch64 virtual address space size
> 
> On 01/02/2019 17:06, Peter Maydell wrote:
>> From: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
>>
>> Since QEMU does not support the ARMv8.2-LVA, Large Virtual Address,
>> extension (yet), the VA address space is 48-bits plus a sign bit. User
>> mode can only handle the positive half of the address space, so that
>> makes a limit of 48 bits.
>>
>> (With LVA, it would be 53 and 52 bits respectively.)
>>
>> The incorrectly large address space conflicts with PAuth instructions,
>> which use bits 48-54 and 56-63 for the pointer authentication code. This
>> also conflicts with (as yet unsupported by QEMU) data tagging and with
>> the ARMv8.5-MTE extension.
>>
>> Signed-off-by: Remi Denis-Courmont <remi.denis.courmont@huawei.com>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  target/arm/cpu.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index 63934a200ad..a68bcc9fedb 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -2512,7 +2512,7 @@ bool write_cpustate_to_list(ARMCPU *cpu);
>>
>>  #if defined(TARGET_AARCH64)
>>  #  define TARGET_PHYS_ADDR_SPACE_BITS 48
>> -#  define TARGET_VIRT_ADDR_SPACE_BITS 64
>> +#  define TARGET_VIRT_ADDR_SPACE_BITS 48
>>  #else
>>  #  define TARGET_PHYS_ADDR_SPACE_BITS 40
>>  #  define TARGET_VIRT_ADDR_SPACE_BITS 32
>>
> 
> This change breaks qemu-aarch64 (using LTP test suite):
> 
> # chroot chroot/arm64/bionic /opt/ltp/testcases/bin/access03
> tst_test.c:1015: INFO: Timeout per run is 0h 05m 00s
> qemu-aarch64: .../qemu/accel/tcg/translate-all.c:2522: page_check_range: Assertion `start < ((target_ulong)1 << L1_MAP_ADDR_SPACE_BITS)' failed.
> qemu:handle_cpu_signal received signal outside vCPU context @ pc=0x60001554
> 
> Any idea?
> Thanks,
> Laurent
>