qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2] linux-user: Undo incomplete mmap


From: Akihiko Odaki
Subject: Re: [PATCH v2] linux-user: Undo incomplete mmap
Date: Thu, 5 Oct 2023 14:17:08 +0900
User-agent: Mozilla Thunderbird

On 2023/10/04 5:42, Richard Henderson wrote:
On 9/21/23 00:09, Akihiko Odaki wrote:
On 2023/09/03 14:39, Akihiko Odaki wrote:
When the host page size is greater than the target page size and
MAP_FIXED or MAP_FIXED_NOREPLACE is requested, mmap will be done for
three parts: start, middle, and end. If a later part of mmap fail,
mmap done in the earlier parts must be reverted.

Fixes: 54936004fd ("mmap emulation")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
V1 -> V2: Rebased.

  linux-user/mmap.c | 65 +++++++++++++++++++++++++++++------------------
  1 file changed, 40 insertions(+), 25 deletions(-)

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 9aab48d4a3..72521f8d27 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -224,13 +224,15 @@ int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
  /* map an incomplete host page */
  static bool mmap_frag(abi_ulong real_start, abi_ulong start, abi_ulong last,
-                      int prot, int flags, int fd, off_t offset)
+                      int prot, int flags, int fd, off_t offset, bool *mapped)
  {
      abi_ulong real_last;
      void *host_start;
      int prot_old, prot_new;
      int host_prot_old, host_prot_new;
+    *mapped = false;
+
      if (!(flags & MAP_ANONYMOUS)
          && (flags & MAP_TYPE) == MAP_SHARED
          && (prot & PROT_WRITE)) {
@@ -271,6 +273,7 @@ static bool mmap_frag(abi_ulong real_start, abi_ulong start, abi_ulong last,
              return false;
          }
          prot_old = prot;
+        *mapped = true;
      }
      prot_new = prot | prot_old;
@@ -448,7 +451,7 @@ abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size, abi_ulong align)
  abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
                       int flags, int fd, off_t offset)
  {
-    abi_ulong ret, last, real_start, real_last, retaddr, host_len;
+    abi_ulong ret, last, real_start, retaddr, host_len;
      abi_ulong passthrough_start = -1, passthrough_last = 0;
      int page_flags;
      off_t host_offset;
@@ -577,12 +580,16 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
          passthrough_start = start;
          passthrough_last = last;
      } else {
+        abi_ulong middle_start = HOST_PAGE_ALIGN(start);
+        abi_ulong middle_last = ((start + len) & qemu_host_page_mask) - 1;
+        abi_ulong mapped_len = 0;
+        bool mapped;
+
          if (start & ~TARGET_PAGE_MASK) {
              errno = EINVAL;
              goto fail;
          }
          last = start + len - 1;
-        real_last = HOST_PAGE_ALIGN(last) - 1;
          /*
           * Test if requested memory area fits target address space
@@ -649,35 +656,26 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
          }
          /* handle the start of the mapping */
-        if (start > real_start) {
-            if (real_last == real_start + qemu_host_page_size - 1) {
+        if (start < middle_start) {
+            if (last < middle_start) {
                  /* one single host page */
                  if (!mmap_frag(real_start, start, last,
-                               target_prot, flags, fd, offset)) {
+                               target_prot, flags, fd, offset, &mapped)) {
                      goto fail;
                  }
                  goto the_end1;
              }
-            if (!mmap_frag(real_start, start,
-                           real_start + qemu_host_page_size - 1,
-                           target_prot, flags, fd, offset)) {
+            if (!mmap_frag(real_start, start, middle_start - 1,
+                           target_prot, flags, fd, offset, &mapped)) {
                  goto fail;
              }
-            real_start += qemu_host_page_size;
-        }
-        /* handle the end of the mapping */
-        if (last < real_last) {
-            abi_ulong real_page = real_last - qemu_host_page_size + 1;
-            if (!mmap_frag(real_page, real_page, last,
-                           target_prot, flags, fd,
-                           offset + real_page - start)) {
-                goto fail;
+            if (mapped) {
+                mapped_len = qemu_host_page_size;
              }
-            real_last -= qemu_host_page_size;
          }
          /* map the middle (easier) */
-        if (real_start < real_last) {
+        if (middle_start < middle_last) {
              void *p, *want_p;
              off_t offset1;
              size_t len1;
@@ -685,10 +683,10 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
              if (flags & MAP_ANONYMOUS) {
                  offset1 = 0;
              } else {
-                offset1 = offset + real_start - start;
+                offset1 = offset + middle_start - start;
              }
-            len1 = real_last - real_start + 1;
-            want_p = g2h_untagged(real_start);
+            len1 = middle_last - middle_start + 1;
+            want_p = g2h_untagged(middle_start);
              p = mmap(want_p, len1, target_to_host_prot(target_prot),
                       flags, fd, offset1);
@@ -697,10 +695,27 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
                      munmap(p, len1);
                      errno = EEXIST;
                  }
+                if (mapped_len) {
+                    munmap(g2h_untagged(middle_start - mapped_len), mapped_len);
+                }
+                goto fail;
+            }
+            mapped_len += len1;
+            passthrough_start = middle_start;
+            passthrough_last = middle_last;
+        }
+
+        /* handle the end of the mapping */
+        if (last > middle_last) {
+            abi_ulong real_page = middle_last + 1;
+            if (!mmap_frag(real_page, real_page, last,
+                           target_prot, flags, fd,
+                           offset + real_page - start, &mapped)) {
+                if (mapped_len) {
+                    munmap(g2h_untagged(real_page - mapped_len), mapped_len);
+                }
                  goto fail;
              }
-            passthrough_start = real_start;
-            passthrough_last = real_last;
          }
      }
   the_end1:

Hi Richard,

Can you have a look at this patch?

munmap isn't always correct -- one has to keep the virtual address space allocated for reserved_va.  So mmap_reserve_or_unmap.

That said, I've got one reorg of mmap.c outstanding already, and I'm planning another to allow use of softmmu in user-only mode.  The latter would eliminate mmap_frag entirely.

That sounds great. I'll wait for the changes instead of pushing this forward.

Regards,
Akihiko Odaki



reply via email to

[Prev in Thread] Current Thread [Next in Thread]