Re: [PATCH v7 00/14] linux-user: brk fixes

[Richard Henderson <richard.henderson@linaro.org>]

On 8/3/23 08:01, Helge Deller wrote:
> * Joel Stanley <joel@jms.id.au>:
>> On Thu, 3 Aug 2023 at 13:55, Helge Deller <deller@gmx.de> wrote:
>>>> 143551 brk(NULL) = 0x0009b000
>>>> 143551 brk(0x0009b8fc) = 0x0009b000
>>>
>>> I think the problem is the brk with 9b000 here.
>>> It's not 64k aligned (=pages size of your ppc64le).
>>>
>>> Please try with this patch on top of Richard's series:
>>>
>>>> @@ -3229,7 +3208,8 @@ static void load_elf_image(const char *image_name, int image_fd,
>>>>        info->end_code = 0;
>>>>        info->start_data = -1;
>>>>        info->end_data = 0;
>>>> -    info->brk = .....
>>> change that to become:
>>>       info->brk = HOST_PAGE_ALIGN(hiaddr);
>>
>> That stopped the crashing, and the binaries seem to run fine. I tested
>> on two hosts: ppc64le (64K) and arm64 (16K).
> 
> Great!
> 
> That made re-read Akihiko's patch:
> ----
> Author: Akihiko Odaki <akihiko.odaki@daynix.com>
>      linux-user: Do not align brk with host page size
> 
>      do_brk() minimizes calls into target_mmap() by aligning the address
>      with host page size, which is potentially larger than the target page
>      size. However, the current implementation of this optimization has two
>      bugs:
> 
>      - The start of brk is rounded up with the host page size while brk
>        advertises an address aligned with the target page size as the
>        beginning of brk. This makes the beginning of brk unmapped.
> ----
> this patch has wrong assumptions.
> 
> The start of brk always needs to be host page aligned.


There is a bunch of code in target_mmap that attempts to manage adjacent guest pages that 
fall into the same host page.  Akihiko's patch assumes that code actually works.  Which I 
think is entirely reasonable.

You can't move brk up like this either (without other adjustments to the binary mapping), 
since that will leave a hole in the guest address space, which can get filled with 
something else later, which will definitely cause problems.


r~