[v2,8/9] bsd-user/mmap.c: Implement MAP_EXCL, required by jemalloc in head

Commit Message

Warner Losh Sept. 22, 2021, 4:56 a.m. UTC

From: Kyle Evans <[e-mail]>

jemalloc requires a working MAP_EXCL. Ensure that no page is double
mapped when specified.

Signed-off-by: Kyle Evans <[e-mail]>
Signed-off-by: Warner Losh <[e-mail]>
---
 bsd-user/mmap.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Richard Henderson Sept. 23, 2021, 5:52 p.m. UTC | #1

On 9/21/21 9:56 PM, Warner Losh wrote:
> +        /* Reject the mapping if any page within the range is mapped */
> +        if (flags & MAP_EXCL) {
> +            for (addr = start; addr < end; addr++) {
> +                if (page_get_flags(addr) != 0)
> +                    goto fail;
> +            }
> +        }

How about

     if ((flags & MAP_EXCL) &&
         page_check_range(start, len, 0) < 0) {
        goto fail;
     }

Hmm.  This (and your page_get_flags check) could assert due to out-of-range guest address. 
  You're currently attempting that,

         /*
          * Test if requested memory area fits target address space
          * It can fail only on 64-bit host with 32-bit target.
          * On any other target/host host mmap() handles this error correctly.
          */
#if TARGET_ABI_BITS == 32 && HOST_LONG_BITS == 64
         if ((unsigned long)start + len - 1 > (abi_ulong) -1) {
             errno = EINVAL;
             goto fail;
         }
#endif

but the test isn't correct.  Note that reserved_va may be applied to 64-bit guests, and 
certainly may be smaller than (abi_ulong)-1.

You want guest_range_valid_untagged here.


r~

Warner Losh Sept. 26, 2021, 6:38 p.m. UTC | #2

On Fri, Sep 24, 2021 at 6:00 AM Richard Henderson <
[e-mail]> wrote:

> On 9/21/21 9:56 PM, Warner Losh wrote:
> > +        /* Reject the mapping if any page within the range is mapped */
> > +        if (flags & MAP_EXCL) {
> > +            for (addr = start; addr < end; addr++) {
> > +                if (page_get_flags(addr) != 0)
> > +                    goto fail;
> > +            }
> > +        }
>
> How about
>
>      if ((flags & MAP_EXCL) &&
>          page_check_range(start, len, 0) < 0) {
>         goto fail;
>      }
>
> Hmm.  This (and your page_get_flags check) could assert due to
> out-of-range guest address.
>   You're currently attempting that,
>
>          /*
>           * Test if requested memory area fits target address space
>           * It can fail only on 64-bit host with 32-bit target.
>           * On any other target/host host mmap() handles this error
> correctly.
>           */
> #if TARGET_ABI_BITS == 32 && HOST_LONG_BITS == 64
>          if ((unsigned long)start + len - 1 > (abi_ulong) -1) {
>              errno = EINVAL;
>              goto fail;
>          }
> #endif
>
> but the test isn't correct.  Note that reserved_va may be applied to
> 64-bit guests, and
> certainly may be smaller than (abi_ulong)-1.
>
> You want guest_range_valid_untagged here.
>

Great! Thanks for the tip!

Warner

Patch

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index 347d314aa9..792ff00548 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -387,7 +387,7 @@  abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size)
 abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
                      int flags, int fd, off_t offset)
 {
-    abi_ulong ret, end, real_start, real_end, retaddr, host_offset, host_len;
+    abi_ulong addr, ret, end, real_start, real_end, retaddr, host_offset, host_len;
 
     mmap_lock();
 #ifdef DEBUG_MMAP
@@ -599,6 +599,14 @@  abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
             goto the_end;
         }
 
+        /* Reject the mapping if any page within the range is mapped */
+        if (flags & MAP_EXCL) {
+            for (addr = start; addr < end; addr++) {
+                if (page_get_flags(addr) != 0)
+                    goto fail;
+            }
+        }
+
         /* handle the start of the mapping */
         if (start > real_start) {
             if (real_end == real_start + qemu_host_page_size) {