| Message ID | 20190201160653.13829-44-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Commit | f6768aa1b4c6a80448eabd22bb9b4123c709caea |
| Headers | show |
| Series | target-arm queue | expand |
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 >
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