Richard Henderson <richard.hender...@linaro.org> writes:
> Ensure that the chosen values for mmap_next_start and
> task_unmapped_base are within the guest address space.
>
> Tested-by: Helge Deller <del...@gmx.de>
> Reviewed-by: Akihiko Odaki <akihiko.od...@daynix.com>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> ---
> linux-user/user-mmap.h | 18 +++++++++++++++++-
> linux-user/main.c | 28 ++++++++++++++++++++++++++++
> linux-user/mmap.c | 18 +++---------------
> 3 files changed, 48 insertions(+), 16 deletions(-)
>
> diff --git a/linux-user/user-mmap.h b/linux-user/user-mmap.h
> index 7265c2c116..fd456e024e 100644
> --- a/linux-user/user-mmap.h
> +++ b/linux-user/user-mmap.h
> @@ -18,6 +18,23 @@
> #ifndef LINUX_USER_USER_MMAP_H
> #define LINUX_USER_USER_MMAP_H
>
> +#if HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 64
> +#ifdef TARGET_AARCH64
> +# define TASK_UNMAPPED_BASE 0x5500000000
> +#else
> +# define TASK_UNMAPPED_BASE (1ul << 38)
> +#endif
> +#else
> +#ifdef TARGET_HPPA
> +# define TASK_UNMAPPED_BASE 0xfa000000
> +#else
> +# define TASK_UNMAPPED_BASE 0x40000000
> +#endif
> +#endif
> +
> +extern abi_ulong task_unmapped_base;
> +extern abi_ulong mmap_next_start;
> +
> int target_mprotect(abi_ulong start, abi_ulong len, int prot);
> abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
> int flags, int fd, off_t offset);
> @@ -26,7 +43,6 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong
> old_size,
> abi_ulong new_size, unsigned long flags,
> abi_ulong new_addr);
> abi_long target_madvise(abi_ulong start, abi_ulong len_in, int advice);
> -extern abi_ulong mmap_next_start;
> abi_ulong mmap_find_vma(abi_ulong, abi_ulong, abi_ulong);
> void mmap_fork_start(void);
> void mmap_fork_end(int child);
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 556956c363..be621dc792 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -821,6 +821,34 @@ int main(int argc, char **argv, char **envp)
> reserved_va = max_reserved_va;
> }
>
> + /*
> + * Temporarily disable
> + * "comparison is always false due to limited range of data type"
> + * due to comparison between (possible) uint64_t and uintptr_t.
> + */
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wtype-limits"
> +
> + /*
> + * Select an initial value for task_unmapped_base that is in range.
> + */
> + if (reserved_va) {
> + if (TASK_UNMAPPED_BASE < reserved_va) {
> + task_unmapped_base = TASK_UNMAPPED_BASE;
> + } else {
> + /* The most common default formula is TASK_SIZE / 3. */
> + task_unmapped_base = TARGET_PAGE_ALIGN(reserved_va / 3);
> + }
> + } else if (TASK_UNMAPPED_BASE < UINTPTR_MAX) {
> + task_unmapped_base = TASK_UNMAPPED_BASE;
> + } else {
> + /* 32-bit host: pick something medium size. */
> + task_unmapped_base = 0x10000000;
> + }
> + mmap_next_start = task_unmapped_base;
> +
> +#pragma GCC diagnostic pop
> +
> {
> Error *err = NULL;
> if (seed_optarg != NULL) {
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index eb04fab8ab..84436d45c8 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -299,20 +299,8 @@ static bool mmap_frag(abi_ulong real_start, abi_ulong
> start, abi_ulong last,
> return true;
> }
>
> -#if HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 64
> -#ifdef TARGET_AARCH64
> -# define TASK_UNMAPPED_BASE 0x5500000000
> -#else
> -# define TASK_UNMAPPED_BASE (1ul << 38)
> -#endif
> -#else
> -#ifdef TARGET_HPPA
> -# define TASK_UNMAPPED_BASE 0xfa000000
> -#else
> -# define TASK_UNMAPPED_BASE 0x40000000
> -#endif
> -#endif
> -abi_ulong mmap_next_start = TASK_UNMAPPED_BASE;
> +abi_ulong task_unmapped_base;
> +abi_ulong mmap_next_start;
I feel we could help ourselves a bit more by documenting these globals
and what they mean:
task_unmapped_base represents the start of unmapped memory in the
guests programs address space. It is generally a function of the size
of the address space and it defined at the start of execution.
mmap_next_start is the base address for the next anonymous mmap and is
increased after each successful map, starting at task_unmapped_base.
One thing I'm slightly confused by is the ELF_ET_DYN_BASE can be above
this (or sometimes the same). Should the mapping of ELF segments be
handled with mmap_next_start? I assume once mmap_next_start meets the
mappings for the ELF segments we skip over until we get to more free
space after the program code?
>
> /*
> * Subroutine of mmap_find_vma, used when we have pre-allocated
> @@ -391,7 +379,7 @@ abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size,
> abi_ulong align)
>
> if ((addr & (align - 1)) == 0) {
> /* Success. */
> - if (start == mmap_next_start && addr >= TASK_UNMAPPED_BASE) {
> + if (start == mmap_next_start && addr >= task_unmapped_base) {
> mmap_next_start = addr + size;
> }
> return addr;
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
