Re: [PATCH] linux-user/mmap.c: check range of mremap result in target address space

[Laurent Vivier]

Le 28/10/2020 à 22:38, Tobias Koch a écrit :
> If mremap succeeds, an additional check is performed to ensure that the
> new address range fits into the target address space. This check was
> previously perfomed in host address space, with the upper bound fixed to
> abi_ulong.
> 
> This patch replaces the static check with a call to `guest_range_valid`,
> performing the range check against the actual size of the target address
> space. It also moves the corresponding block to prevent it from being
> called incorrectly when the mapping itself fails.
> 
> Signed-off-by: Tobias Koch <tobias.koch@nonterra.com>
> ---
>  linux-user/mmap.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index f261563420..101bd013a1 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -751,20 +751,23 @@ abi_long target_mremap(abi_ulong old_addr, abi_ulong 
> old_size,
>          }
>          if (prot == 0) {
>              host_addr = mremap(g2h(old_addr), old_size, new_size, flags);
> -            if (host_addr != MAP_FAILED && reserved_va && old_size > 
> new_size) {
> -                mmap_reserve(old_addr + old_size, old_size - new_size);
> +
> +            if (host_addr != MAP_FAILED) {
> +                /* Check if address fits target address space */
> +                if (!guest_range_valid(h2g(host_addr), new_size)) {
> +                    /* Revert mremap() changes */
> +                    host_addr = mremap(g2h(old_addr), new_size, old_size,
> +                                       flags);
> +                    errno = ENOMEM;
> +                    host_addr = MAP_FAILED;
> +                } else if (reserved_va && old_size > new_size) {
> +                    mmap_reserve(old_addr + old_size, old_size - new_size);
> +                }
>              }
>          } else {
>              errno = ENOMEM;
>              host_addr = MAP_FAILED;
>          }
> -        /* Check if address fits target address space */
> -        if ((unsigned long)host_addr + new_size > (abi_ulong)-1) {
> -            /* Revert mremap() changes */
> -            host_addr = mremap(g2h(old_addr), new_size, old_size, flags);
> -            errno = ENOMEM;
> -            host_addr = MAP_FAILED;
> -        }
>      }
>  
>      if (host_addr == MAP_FAILED) {
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>