diff mbox series

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

Message ID 20201028213833.26592-1-tobias.koch@nonterra.com
State New, archived
Headers show
Series linux-user/mmap.c: check range of mremap result in target address space | expand

Commit Message

Tobias Koch Oct. 28, 2020, 9:38 p.m. UTC
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(-)

Comments

Laurent Vivier Nov. 26, 2020, 7:44 a.m. UTC | #1
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>
diff mbox series

Patch

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) {