Re: [PULL v2 05/13] accel/tcg: Relax va restrictions on 64-bit guests
On 6/5/20 10:46 AM, Alex Bennée wrote:
>
> Richard Henderson writes:
>
>> On 6/5/20 7:11 AM, Alex Bennée wrote:
>>> @@ -467,7 +467,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len,
>>> int prot,
>>> * It can fail only on 64-bit host with 32-bit target.
>>> * On any other target/host host mmap() handles this error
>>> correctly.
>>> */
>>> -if (!guest_range_valid(start, len)) {
>>> +if (end < start || !guest_range_valid(start, len)) {
>>> errno = ENOMEM;
>>> goto fail;
>>> }
>>
>> Interesting. I was adjusting guest_range_valid tagged pointers yesterday,
>> and
>> thought that it looked buggy.
>
> Should be picking this up in guest_range_valid?
I think so. How can a range really be considered valid if it wraps?
r~
Re: [PULL v2 05/13] accel/tcg: Relax va restrictions on 64-bit guests
Richard Henderson writes:
> On 6/5/20 7:11 AM, Alex Bennée wrote:
>> @@ -467,7 +467,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int
>> prot,
>> * It can fail only on 64-bit host with 32-bit target.
>> * On any other target/host host mmap() handles this error
>> correctly.
>> */
>> -if (!guest_range_valid(start, len)) {
>> +if (end < start || !guest_range_valid(start, len)) {
>> errno = ENOMEM;
>> goto fail;
>> }
>
> Interesting. I was adjusting guest_range_valid tagged pointers yesterday, and
> thought that it looked buggy.
Should be picking this up in guest_range_valid?
--
Alex Bennée
Re: [PULL v2 05/13] accel/tcg: Relax va restrictions on 64-bit guests
On 6/5/20 7:11 AM, Alex Bennée wrote:
> @@ -467,7 +467,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int
> prot,
> * It can fail only on 64-bit host with 32-bit target.
> * On any other target/host host mmap() handles this error correctly.
> */
> -if (!guest_range_valid(start, len)) {
> +if (end < start || !guest_range_valid(start, len)) {
> errno = ENOMEM;
> goto fail;
> }
Interesting. I was adjusting guest_range_valid tagged pointers yesterday, and
thought that it looked buggy.
r~
Re: [PULL v2 05/13] accel/tcg: Relax va restrictions on 64-bit guests
Alex Bennée writes:
> Laurent Vivier writes:
>
>> On 15/05/2020 16:43, Alex Bennée wrote:
>>> From: Richard Henderson
>>>
>>> We cannot at present limit a 64-bit guest to a virtual address
>>> space smaller than the host. It will mostly work to ignore this
>>> limitation, except if the guest uses high bits of the address
>>> space for tags. But it will certainly work better, as presently
>>> we can wind up failing to allocate the guest stack.
>>>
>>> Widen our user-only page tree to the host or abi pointer width.
>>> Remove the workaround for this problem from target/alpha.
>>> Always validate guest addresses vs reserved_va, as there we
>>> control allocation ourselves.
>>>
>>> Signed-off-by: Richard Henderson
>>> Signed-off-by: Alex Bennée
>>>
>>> Message-Id: <20200513175134.19619-7-alex.ben...@linaro.org>
>>>
>>
>> This patch breaks a test case in LTP with 64bit targets on x86_64 host:
>>
>> sudo linux-user/mips64el-linux-user/qemu-mips64el \
>> -L chroot/mips64el/stretch/ \
>> chroot/mips64el/stretch/opt/ltp/testcases/bin/mmap15
>>
>> qemu-mips64el: accel/tcg/translate-all.c:2533: page_set_flags: Assertion
>> `start < end' failed.
>> qemu:handle_cpu_signal received signal outside vCPU context @
>> pc=0x7f0015f6e7cb
>>
>> Could you have a look?
>
> Can confirm I've replicated:
>
> 18:30:20 [alex.bennee@hackbox2:~/l/q/b/user.static] next/various-fixes|✔ 32
> +
> sudo ./mips64el-linux-user/qemu-mips64el -L
> ~/lsrc/buildroot.git/builds/mips64el/target/
> ~/lsrc/buildroot.git/builds/mips64el/target/usr/lib/ltp-testsuite/testcases/bin/mmap
> 15
> [sudo] password for alex.bennee:
> qemu-mips64el:
> /home/alex.bennee/lsrc/qemu.git/accel/tcg/translate-all.c:2533:
> page_set_flags: Assertion `start < end' failed.
> qemu:handle_cpu_signal received signal outside vCPU context @ pc=0x6c28c2
>
> Also TIL you can use buildroot to build ltp ;-)
I'm not sure how the change has tripped this failure but it seems to be
a simple overflow. The following fixes it but I'm curious about the
formulation of guest_addr_valid and why it's written that way:
modified linux-user/mmap.c
@@ -467,7 +467,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int
prot,
* It can fail only on 64-bit host with 32-bit target.
* On any other target/host host mmap() handles this error correctly.
*/
-if (!guest_range_valid(start, len)) {
+if (end < start || !guest_range_valid(start, len)) {
errno = ENOMEM;
goto fail;
}
>
>>
>> Thanks,
>> Laurent
--
Alex Bennée
Re: [PULL v2 05/13] accel/tcg: Relax va restrictions on 64-bit guests
Laurent Vivier writes: > On 15/05/2020 16:43, Alex Bennée wrote: >> From: Richard Henderson >> >> We cannot at present limit a 64-bit guest to a virtual address >> space smaller than the host. It will mostly work to ignore this >> limitation, except if the guest uses high bits of the address >> space for tags. But it will certainly work better, as presently >> we can wind up failing to allocate the guest stack. >> >> Widen our user-only page tree to the host or abi pointer width. >> Remove the workaround for this problem from target/alpha. >> Always validate guest addresses vs reserved_va, as there we >> control allocation ourselves. >> >> Signed-off-by: Richard Henderson >> Signed-off-by: Alex Bennée >> >> Message-Id: <20200513175134.19619-7-alex.ben...@linaro.org> >> > > This patch breaks a test case in LTP with 64bit targets on x86_64 host: > > sudo linux-user/mips64el-linux-user/qemu-mips64el \ > -L chroot/mips64el/stretch/ \ > chroot/mips64el/stretch/opt/ltp/testcases/bin/mmap15 > > qemu-mips64el: accel/tcg/translate-all.c:2533: page_set_flags: Assertion > `start < end' failed. > qemu:handle_cpu_signal received signal outside vCPU context @ > pc=0x7f0015f6e7cb > > Could you have a look? Can confirm I've replicated: 18:30:20 [alex.bennee@hackbox2:~/l/q/b/user.static] next/various-fixes|✔ 32 + sudo ./mips64el-linux-user/qemu-mips64el -L ~/lsrc/buildroot.git/builds/mips64el/target/ ~/lsrc/buildroot.git/builds/mips64el/target/usr/lib/ltp-testsuite/testcases/bin/mmap 15 [sudo] password for alex.bennee: qemu-mips64el: /home/alex.bennee/lsrc/qemu.git/accel/tcg/translate-all.c:2533: page_set_flags: Assertion `start < end' failed. qemu:handle_cpu_signal received signal outside vCPU context @ pc=0x6c28c2 Also TIL you can use buildroot to build ltp ;-) > > Thanks, > Laurent -- Alex Bennée
Re: [PULL v2 05/13] accel/tcg: Relax va restrictions on 64-bit guests
On 15/05/2020 16:43, Alex Bennée wrote: > From: Richard Henderson > > We cannot at present limit a 64-bit guest to a virtual address > space smaller than the host. It will mostly work to ignore this > limitation, except if the guest uses high bits of the address > space for tags. But it will certainly work better, as presently > we can wind up failing to allocate the guest stack. > > Widen our user-only page tree to the host or abi pointer width. > Remove the workaround for this problem from target/alpha. > Always validate guest addresses vs reserved_va, as there we > control allocation ourselves. > > Signed-off-by: Richard Henderson > Signed-off-by: Alex Bennée > > Message-Id: <20200513175134.19619-7-alex.ben...@linaro.org> > This patch breaks a test case in LTP with 64bit targets on x86_64 host: sudo linux-user/mips64el-linux-user/qemu-mips64el \ -L chroot/mips64el/stretch/ \ chroot/mips64el/stretch/opt/ltp/testcases/bin/mmap15 qemu-mips64el: accel/tcg/translate-all.c:2533: page_set_flags: Assertion `start < end' failed. qemu:handle_cpu_signal received signal outside vCPU context @ pc=0x7f0015f6e7cb Could you have a look? Thanks, Laurent
