qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] bsd-user mmap fixes
@ 2021-09-17  2:56 Warner Losh
  2021-09-17  2:56 ` [PATCH 1/9] bsd-user: Apply e6deac9cf99 from linux-user (zero anonymous memory) Warner Losh
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Warner Losh @ 2021-09-17  2:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: kevans, Warner Losh

This series synchronizes mmap.c with the bsd-user fork. This is a mix of old bug
fixes pulled in from linux-user, as well as some newer fixes to adress bugs
found in check-tcg and recent FreeBSD developments. There are also a couple of
style commits.

Guy Yur (1):
  bsd-user: Don't try to mmap fd when it is -1 independently from
    MAP_ANONYMOUS flag

Kyle Evans (1):
  bsd-user: Implement MAP_EXCL, required by jemalloc in head

Mikaël Urankar (2):
  bsd-user: Apply e6deac9cf99 from linux-user (zero anonymous memory)
  bsd-user: Apply fb7e378cf9c from linux-user (fix FORTIFY warnings)

Paolo Bonzini (1):
  bsd-user: Apply 86abac06c14 from linux-user (target_mprotect can't
    fail)

Warner Losh (4):
  bsd-user: MAP_ symbols are defined, so no need for ifdefs
  bsd-user: mmap return ENOMEM on overflow
  bsd-user: mmap prefer MAP_ANON for BSD
  bsd-user: mmap line wrap change

 bsd-user/mmap.c | 69 +++++++++++++++++++++++++------------------------
 1 file changed, 35 insertions(+), 34 deletions(-)

-- 
2.32.0



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/9] bsd-user: Apply e6deac9cf99 from linux-user (zero anonymous memory)
  2021-09-17  2:56 [PATCH 0/9] bsd-user mmap fixes Warner Losh
@ 2021-09-17  2:56 ` Warner Losh
  2021-09-17 15:02   ` Philippe Mathieu-Daudé
  2021-09-17  2:56 ` [PATCH 2/9] bsd-user: Apply fb7e378cf9c from linux-user (fix FORTIFY warnings) Warner Losh
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Warner Losh @ 2021-09-17  2:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Chen Gang, kevans, Riku Voipio, Laurent Vivier, Warner Losh,
	Mikaël Urankar

From: Mikaël Urankar <mikael.urankar@gmail.com>

linux-user/mmap.c: Always zero MAP_ANONYMOUS memory in mmap_frag()

When mapping MAP_ANONYMOUS memory fragments, still need notice about to
set it zero, or it will cause issues.

Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Riku Voipio <riku.voipio@linaro.org>
[ bsd-user merge by Mikaël Urankar, updated for untagged by Warner Losh ]
Signed-off-by: Mikaël Urankar <mikael.urankar@gmail.com>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/mmap.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index b40ab9045f..fc3c1480f5 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -180,10 +180,12 @@ static int mmap_frag(abi_ulong real_start,
         if (prot_new != (prot1 | PROT_WRITE))
             mprotect(host_start, qemu_host_page_size, prot_new);
     } else {
-        /* just update the protection */
         if (prot_new != prot1) {
             mprotect(host_start, qemu_host_page_size, prot_new);
         }
+        if (prot_new & PROT_WRITE) {
+            memset(g2h_untagged(start), 0, end - start);
+        }
     }
     return 0;
 }
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/9] bsd-user: Apply fb7e378cf9c from linux-user (fix FORTIFY warnings)
  2021-09-17  2:56 [PATCH 0/9] bsd-user mmap fixes Warner Losh
  2021-09-17  2:56 ` [PATCH 1/9] bsd-user: Apply e6deac9cf99 from linux-user (zero anonymous memory) Warner Losh
@ 2021-09-17  2:56 ` Warner Losh
  2021-09-17  2:56 ` [PATCH 3/9] bsd-user: MAP_ symbols are defined, so no need for ifdefs Warner Losh
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Warner Losh @ 2021-09-17  2:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Anthony Liguori, Juan Quintela, kevans, Warner Losh,
	Kirill A . Shutemov, Mikaël Urankar

From: Mikaël Urankar <mikael.urankar@gmail.com>

linux-user/mmap.c: fix warnings with _FORTIFY_SOURCE

CC    i386-linux-user/mmap.o
cc1: warnings being treated as errors
/usr/src/RPM/BUILD/qemu-0.11.92/linux-user/mmap.c: In function 'mmap_frag':
/usr/src/RPM/BUILD/qemu-0.11.92/linux-user/mmap.c:253: error: ignoring return value of 'pread', declared with attribute warn_unused_result
/usr/src/RPM/BUILD/qemu-0.11.92/linux-user/mmap.c: In function 'target_mmap':
/usr/src/RPM/BUILD/qemu-0.11.92/linux-user/mmap.c:477: error: ignoring return value of 'pread', declared with attribute warn_unused_result
make[1]: *** [mmap.o] Error 1

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
Signed-off-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
[ Merged to bsd-user by Mikaël Urankared updated by Warner Losh for untagged ]
Signed-off-by: Mikaël Urankar <mikael.urankar@gmail.com>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/mmap.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index fc3c1480f5..90b6313161 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -174,7 +174,8 @@ static int mmap_frag(abi_ulong real_start,
             mprotect(host_start, qemu_host_page_size, prot1 | PROT_WRITE);
 
         /* read the corresponding file data */
-        pread(fd, g2h_untagged(start), end - start, offset);
+        if (pread(fd, g2h_untagged(start), end - start, offset) == -1)
+            return -1;
 
         /* put final protection */
         if (prot_new != (prot1 | PROT_WRITE))
@@ -593,7 +594,8 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
                                   -1, 0);
             if (retaddr == -1)
                 goto fail;
-            pread(fd, g2h_untagged(start), len, offset);
+            if (pread(fd, g2h_untagged(start), len, offset) == -1)
+                goto fail;
             if (!(prot & PROT_WRITE)) {
                 ret = target_mprotect(start, len, prot);
                 if (ret != 0) {
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/9] bsd-user: MAP_ symbols are defined, so no need for ifdefs
  2021-09-17  2:56 [PATCH 0/9] bsd-user mmap fixes Warner Losh
  2021-09-17  2:56 ` [PATCH 1/9] bsd-user: Apply e6deac9cf99 from linux-user (zero anonymous memory) Warner Losh
  2021-09-17  2:56 ` [PATCH 2/9] bsd-user: Apply fb7e378cf9c from linux-user (fix FORTIFY warnings) Warner Losh
@ 2021-09-17  2:56 ` Warner Losh
  2021-09-17 15:03   ` Philippe Mathieu-Daudé
  2021-09-17  2:56 ` [PATCH 4/9] bsd-user: mmap return ENOMEM on overflow Warner Losh
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Warner Losh @ 2021-09-17  2:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: kevans, Warner Losh

All these MAP_ symbols are always defined on supported FreeBSD versions
(12.2 and newer), so remove the #ifdefs since they aren't needed.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/mmap.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index 90b6313161..c40059d7fc 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -285,13 +285,9 @@ static abi_ulong mmap_find_vma_aligned(abi_ulong start, abi_ulong size,
     wrapped = repeat = 0;
     prev = 0;
     flags = MAP_ANONYMOUS | MAP_PRIVATE;
-#ifdef MAP_ALIGNED
     if (alignment != 0) {
         flags |= MAP_ALIGNED(alignment);
     }
-#else
-    /* XXX TODO */
-#endif
 
     for (;; prev = ptr) {
         /*
@@ -406,22 +402,18 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
             printf("MAP_ALIGNED(%u) ", (flags & MAP_ALIGNMENT_MASK)
                     >> MAP_ALIGNMENT_SHIFT);
         }
-#if MAP_GUARD
         if (flags & MAP_GUARD) {
             printf("MAP_GUARD ");
         }
-#endif
         if (flags & MAP_FIXED) {
             printf("MAP_FIXED ");
         }
         if (flags & MAP_ANONYMOUS) {
             printf("MAP_ANON ");
         }
-#ifdef MAP_EXCL
         if (flags & MAP_EXCL) {
             printf("MAP_EXCL ");
         }
-#endif
         if (flags & MAP_PRIVATE) {
             printf("MAP_PRIVATE ");
         }
@@ -431,11 +423,9 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         if (flags & MAP_NOCORE) {
             printf("MAP_NOCORE ");
         }
-#ifdef MAP_STACK
         if (flags & MAP_STACK) {
             printf("MAP_STACK ");
         }
-#endif
         printf("fd=%d offset=0x%llx\n", fd, offset);
     }
 #endif
@@ -444,7 +434,6 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         errno = EINVAL;
         goto fail;
     }
-#ifdef MAP_STACK
     if (flags & MAP_STACK) {
         if ((fd != -1) || ((prot & (PROT_READ | PROT_WRITE)) !=
                     (PROT_READ | PROT_WRITE))) {
@@ -452,8 +441,6 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
             goto fail;
         }
     }
-#endif /* MAP_STACK */
-#ifdef MAP_GUARD
     if ((flags & MAP_GUARD) && (prot != PROT_NONE || fd != -1 ||
         offset != 0 || (flags & (MAP_SHARED | MAP_PRIVATE |
         /* MAP_PREFAULT | */ /* MAP_PREFAULT not in mman.h */
@@ -461,7 +448,6 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         errno = EINVAL;
         goto fail;
     }
-#endif
 
     if (offset & ~TARGET_PAGE_MASK) {
         errno = EINVAL;
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/9] bsd-user: mmap return ENOMEM on overflow
  2021-09-17  2:56 [PATCH 0/9] bsd-user mmap fixes Warner Losh
                   ` (2 preceding siblings ...)
  2021-09-17  2:56 ` [PATCH 3/9] bsd-user: MAP_ symbols are defined, so no need for ifdefs Warner Losh
@ 2021-09-17  2:56 ` Warner Losh
  2021-09-17  2:56 ` [PATCH 5/9] bsd-user: mmap prefer MAP_ANON for BSD Warner Losh
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Warner Losh @ 2021-09-17  2:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: kevans, Warner Losh

mmap should return ENOMEM on len overflow rather than EINVAL. Return
EINVAL when len == 0 and ENOMEM when the rounded to a page length is 0.
Found by make check-tcg.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/mmap.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index c40059d7fc..0acc2db712 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -454,11 +454,18 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         goto fail;
     }
 
-    len = TARGET_PAGE_ALIGN(len);
     if (len == 0) {
         errno = EINVAL;
         goto fail;
     }
+
+    /* Check for overflows */
+    len = TARGET_PAGE_ALIGN(len);
+    if (len == 0) {
+        errno = ENOMEM;
+        goto fail;
+    }
+
     real_start = start & qemu_host_page_mask;
     host_offset = offset & qemu_host_page_mask;
 
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/9] bsd-user: mmap prefer MAP_ANON for BSD
  2021-09-17  2:56 [PATCH 0/9] bsd-user mmap fixes Warner Losh
                   ` (3 preceding siblings ...)
  2021-09-17  2:56 ` [PATCH 4/9] bsd-user: mmap return ENOMEM on overflow Warner Losh
@ 2021-09-17  2:56 ` Warner Losh
  2021-09-17 15:04   ` Philippe Mathieu-Daudé
  2021-09-17  2:56 ` [PATCH 6/9] bsd-user: mmap line wrap change Warner Losh
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Warner Losh @ 2021-09-17  2:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: kevans, Warner Losh

MAP_ANON and MAP_ANONYMOUS are identical. Prefer MAP_ANON for BSD since
the file is now a confusing mix of the two.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/mmap.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index 0acc2db712..bafbdacd31 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -284,7 +284,7 @@ static abi_ulong mmap_find_vma_aligned(abi_ulong start, abi_ulong size,
     addr = start;
     wrapped = repeat = 0;
     prev = 0;
-    flags = MAP_ANONYMOUS | MAP_PRIVATE;
+    flags = MAP_ANON | MAP_PRIVATE;
     if (alignment != 0) {
         flags |= MAP_ALIGNED(alignment);
     }
@@ -408,7 +408,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         if (flags & MAP_FIXED) {
             printf("MAP_FIXED ");
         }
-        if (flags & MAP_ANONYMOUS) {
+        if (flags & MAP_ANON) {
             printf("MAP_ANON ");
         }
         if (flags & MAP_EXCL) {
@@ -430,7 +430,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
     }
 #endif
 
-    if ((flags & MAP_ANONYMOUS) && fd != -1) {
+    if ((flags & MAP_ANON) && fd != -1) {
         errno = EINVAL;
         goto fail;
     }
@@ -532,7 +532,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
          * qemu_real_host_page_size
          */
         p = mmap(g2h_untagged(start), host_len, prot,
-                 flags | MAP_FIXED | ((fd != -1) ? MAP_ANONYMOUS : 0), -1, 0);
+                 flags | MAP_FIXED | ((fd != -1) ? MAP_ANON : 0), -1, 0);
         if (p == MAP_FAILED)
             goto fail;
         /* update start so that it points to the file position at 'offset' */
@@ -694,8 +694,7 @@ static void mmap_reserve(abi_ulong start, abi_ulong size)
     }
     if (real_start != real_end) {
         mmap(g2h_untagged(real_start), real_end - real_start, PROT_NONE,
-                 MAP_FIXED | MAP_ANONYMOUS | MAP_PRIVATE,
-                 -1, 0);
+                 MAP_FIXED | MAP_ANON | MAP_PRIVATE, -1, 0);
     }
 }
 
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 6/9] bsd-user: mmap line wrap change
  2021-09-17  2:56 [PATCH 0/9] bsd-user mmap fixes Warner Losh
                   ` (4 preceding siblings ...)
  2021-09-17  2:56 ` [PATCH 5/9] bsd-user: mmap prefer MAP_ANON for BSD Warner Losh
@ 2021-09-17  2:56 ` Warner Losh
  2021-09-17  2:56 ` [PATCH 7/9] bsd-user: Don't try to mmap fd when it is -1 independently from MAP_ANONYMOUS flag Warner Losh
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Warner Losh @ 2021-09-17  2:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: kevans, Warner Losh

Keep the shifted expression on one line. It's the same number of lines
and easier to read like this.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/mmap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index bafbdacd31..8b763fffc3 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -399,8 +399,8 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
                prot & PROT_WRITE ? 'w' : '-',
                prot & PROT_EXEC ? 'x' : '-');
         if (flags & MAP_ALIGNMENT_MASK) {
-            printf("MAP_ALIGNED(%u) ", (flags & MAP_ALIGNMENT_MASK)
-                    >> MAP_ALIGNMENT_SHIFT);
+            printf("MAP_ALIGNED(%u) ",
+                   (flags & MAP_ALIGNMENT_MASK) >> MAP_ALIGNMENT_SHIFT);
         }
         if (flags & MAP_GUARD) {
             printf("MAP_GUARD ");
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 7/9] bsd-user: Don't try to mmap fd when it is -1 independently from MAP_ANONYMOUS flag
  2021-09-17  2:56 [PATCH 0/9] bsd-user mmap fixes Warner Losh
                   ` (5 preceding siblings ...)
  2021-09-17  2:56 ` [PATCH 6/9] bsd-user: mmap line wrap change Warner Losh
@ 2021-09-17  2:56 ` Warner Losh
  2021-09-17  2:58   ` Warner Losh
  2021-09-17  2:56 ` [PATCH 8/9] bsd-user: Implement MAP_EXCL, required by jemalloc in head Warner Losh
  2021-09-17  2:56 ` [PATCH 9/9] bsd-user: Apply 86abac06c14 from linux-user (target_mprotect can't fail) Warner Losh
  8 siblings, 1 reply; 15+ messages in thread
From: Warner Losh @ 2021-09-17  2:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: kevans, Guy Yur, Warner Losh, Guy Yur

From: Guy Yur <guyyur@ngmail.com>

Switch checks for !(flags & MAP_ANONYMOUS) with checks for fd != -1.
MAP_STACK and MAP_GUARD also force fd == -1 and they don't require
mapping the fd either.

Signed-off-by: Guy Yur <guyyur@gmail.com>
[ partially merged before, finishing the job and documenting origin]
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/mmap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index 8b763fffc3..347d314aa9 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -154,7 +154,7 @@ static int mmap_frag(abi_ulong real_start,
     if (prot1 == 0) {
         /* no page was there, so we allocate one */
         void *p = mmap(host_start, qemu_host_page_size, prot,
-                       flags | MAP_ANON, -1, 0);
+                       flags | ((fd != -1) ? MAP_ANON : 0), -1, 0);
         if (p == MAP_FAILED)
             return -1;
         prot1 = prot;
@@ -162,7 +162,7 @@ static int mmap_frag(abi_ulong real_start,
     prot1 &= PAGE_BITS;
 
     prot_new = prot | prot1;
-    if (!(flags & MAP_ANON)) {
+    if (fd != -1) {
         /* msync() won't work here, so we return an error if write is
            possible while it is a shared mapping */
         if ((flags & TARGET_BSD_MAP_FLAGMASK) == MAP_SHARED &&
@@ -571,7 +571,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
          * worst case: we cannot map the file because the offset is not
          * aligned, so we read it
          */
-        if (!(flags & MAP_ANON) &&
+        if (fd != -1 &&
             (offset & ~qemu_host_page_mask) != (start & ~qemu_host_page_mask)) {
             /*
              * msync() won't work here, so we return an error if write is
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 8/9] bsd-user: Implement MAP_EXCL, required by jemalloc in head
  2021-09-17  2:56 [PATCH 0/9] bsd-user mmap fixes Warner Losh
                   ` (6 preceding siblings ...)
  2021-09-17  2:56 ` [PATCH 7/9] bsd-user: Don't try to mmap fd when it is -1 independently from MAP_ANONYMOUS flag Warner Losh
@ 2021-09-17  2:56 ` Warner Losh
  2021-09-17  2:56 ` [PATCH 9/9] bsd-user: Apply 86abac06c14 from linux-user (target_mprotect can't fail) Warner Losh
  8 siblings, 0 replies; 15+ messages in thread
From: Warner Losh @ 2021-09-17  2:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: Kyle Evans, Warner Losh

From: Kyle Evans <kevans@FreeBSD.org>

jemalloc requires a working MAP_EXCL. Emulate it by ensuring we don't
double map anything.

Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/mmap.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

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



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 9/9] bsd-user: Apply 86abac06c14 from linux-user (target_mprotect can't fail)
  2021-09-17  2:56 [PATCH 0/9] bsd-user mmap fixes Warner Losh
                   ` (7 preceding siblings ...)
  2021-09-17  2:56 ` [PATCH 8/9] bsd-user: Implement MAP_EXCL, required by jemalloc in head Warner Losh
@ 2021-09-17  2:56 ` Warner Losh
  8 siblings, 0 replies; 15+ messages in thread
From: Warner Losh @ 2021-09-17  2:56 UTC (permalink / raw)
  To: qemu-devel; +Cc: kevans, Riku Voipio, Warner Losh, Paolo Bonzini

From: Paolo Bonzini <pbonzini@redhat.com>

linux-user: assert that target_mprotect cannot fail

All error conditions that target_mprotect checks are also checked
by target_mmap.  EACCESS cannot happen because we are just removing
PROT_WRITE.  ENOMEM should not happen because we are modifying a
whole VMA (and we have bigger problems anyway if it happens).

Fixes a Coverity false positive, where Coverity complains about
target_mprotect's return value being passed to tb_invalidate_phys_range.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Riku Voipio <riku.voipio@linaro.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/mmap.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index 792ff00548..4ddbd50b62 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -591,10 +591,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
                 goto fail;
             if (!(prot & PROT_WRITE)) {
                 ret = target_mprotect(start, len, prot);
-                if (ret != 0) {
-                    start = ret;
-                    goto the_end;
-                }
+                assert(ret == 0);
             }
             goto the_end;
         }
-- 
2.32.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 7/9] bsd-user: Don't try to mmap fd when it is -1 independently from MAP_ANONYMOUS flag
  2021-09-17  2:56 ` [PATCH 7/9] bsd-user: Don't try to mmap fd when it is -1 independently from MAP_ANONYMOUS flag Warner Losh
@ 2021-09-17  2:58   ` Warner Losh
  0 siblings, 0 replies; 15+ messages in thread
From: Warner Losh @ 2021-09-17  2:58 UTC (permalink / raw)
  To: QEMU Developers; +Cc: Kyle Evans, Guy Yur

[-- Attachment #1: Type: text/plain, Size: 2038 bytes --]

On Thu, Sep 16, 2021 at 8:56 PM Warner Losh <imp@bsdimp.com> wrote:

> From: Guy Yur <guyyur@ngmail.com>
>

I need to fix this email address in the next round or for the pull request.
It's gmail.com, not ngmail.com.

Switch checks for !(flags & MAP_ANONYMOUS) with checks for fd != -1.
> MAP_STACK and MAP_GUARD also force fd == -1 and they don't require
> mapping the fd either.
>
> Signed-off-by: Guy Yur <guyyur@gmail.com>
> [ partially merged before, finishing the job and documenting origin]
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/mmap.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> index 8b763fffc3..347d314aa9 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -154,7 +154,7 @@ static int mmap_frag(abi_ulong real_start,
>      if (prot1 == 0) {
>          /* no page was there, so we allocate one */
>          void *p = mmap(host_start, qemu_host_page_size, prot,
> -                       flags | MAP_ANON, -1, 0);
> +                       flags | ((fd != -1) ? MAP_ANON : 0), -1, 0);
>          if (p == MAP_FAILED)
>              return -1;
>          prot1 = prot;
> @@ -162,7 +162,7 @@ static int mmap_frag(abi_ulong real_start,
>      prot1 &= PAGE_BITS;
>
>      prot_new = prot | prot1;
> -    if (!(flags & MAP_ANON)) {
> +    if (fd != -1) {
>          /* msync() won't work here, so we return an error if write is
>             possible while it is a shared mapping */
>          if ((flags & TARGET_BSD_MAP_FLAGMASK) == MAP_SHARED &&
> @@ -571,7 +571,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len,
> int prot,
>           * worst case: we cannot map the file because the offset is not
>           * aligned, so we read it
>           */
> -        if (!(flags & MAP_ANON) &&
> +        if (fd != -1 &&
>              (offset & ~qemu_host_page_mask) != (start &
> ~qemu_host_page_mask)) {
>              /*
>               * msync() won't work here, so we return an error if write is
> --
> 2.32.0
>
>

[-- Attachment #2: Type: text/html, Size: 3025 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/9] bsd-user: Apply e6deac9cf99 from linux-user (zero anonymous memory)
  2021-09-17  2:56 ` [PATCH 1/9] bsd-user: Apply e6deac9cf99 from linux-user (zero anonymous memory) Warner Losh
@ 2021-09-17 15:02   ` Philippe Mathieu-Daudé
  2021-09-17 15:10     ` Warner Losh
  0 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-17 15:02 UTC (permalink / raw)
  To: Warner Losh, qemu-devel
  Cc: kevans, Riku Voipio, Chen Gang, Mikaël Urankar,
	Laurent Vivier

On 9/17/21 4:56 AM, Warner Losh wrote:
> From: Mikaël Urankar <mikael.urankar@gmail.com>
> 
> linux-user/mmap.c: Always zero MAP_ANONYMOUS memory in mmap_frag()

Please use it as subject, "bsd-user/mmap: Always zero MAP_ANONYMOUS
memory in mmap_frag()"

Then describe:

Similar to the equivalent linux-user commit e6deac9cf99, ...

> 
> When mapping MAP_ANONYMOUS memory fragments, still need notice about to
> set it zero, or it will cause issues.
> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Riku Voipio <riku.voipio@linaro.org>

^ These tags were for another file, not this one, please
remove them.

> [ bsd-user merge by Mikaël Urankar, updated for untagged by Warner Losh ]
> Signed-off-by: Mikaël Urankar <mikael.urankar@gmail.com>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/mmap.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> index b40ab9045f..fc3c1480f5 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -180,10 +180,12 @@ static int mmap_frag(abi_ulong real_start,
>          if (prot_new != (prot1 | PROT_WRITE))
>              mprotect(host_start, qemu_host_page_size, prot_new);
>      } else {
> -        /* just update the protection */
>          if (prot_new != prot1) {
>              mprotect(host_start, qemu_host_page_size, prot_new);
>          }
> +        if (prot_new & PROT_WRITE) {
> +            memset(g2h_untagged(start), 0, end - start);
> +        }
>      }
>      return 0;
>  }
> 



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/9] bsd-user: MAP_ symbols are defined, so no need for ifdefs
  2021-09-17  2:56 ` [PATCH 3/9] bsd-user: MAP_ symbols are defined, so no need for ifdefs Warner Losh
@ 2021-09-17 15:03   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-17 15:03 UTC (permalink / raw)
  To: Warner Losh, qemu-devel; +Cc: kevans

On 9/17/21 4:56 AM, Warner Losh wrote:
> All these MAP_ symbols are always defined on supported FreeBSD versions
> (12.2 and newer), so remove the #ifdefs since they aren't needed.
> 
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/mmap.c | 14 --------------
>  1 file changed, 14 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 5/9] bsd-user: mmap prefer MAP_ANON for BSD
  2021-09-17  2:56 ` [PATCH 5/9] bsd-user: mmap prefer MAP_ANON for BSD Warner Losh
@ 2021-09-17 15:04   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-17 15:04 UTC (permalink / raw)
  To: Warner Losh, qemu-devel; +Cc: kevans

On 9/17/21 4:56 AM, Warner Losh wrote:
> MAP_ANON and MAP_ANONYMOUS are identical. Prefer MAP_ANON for BSD since
> the file is now a confusing mix of the two.
> 
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/mmap.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/9] bsd-user: Apply e6deac9cf99 from linux-user (zero anonymous memory)
  2021-09-17 15:02   ` Philippe Mathieu-Daudé
@ 2021-09-17 15:10     ` Warner Losh
  0 siblings, 0 replies; 15+ messages in thread
From: Warner Losh @ 2021-09-17 15:10 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Chen Gang, Kyle Evans, Riku Voipio, Laurent Vivier,
	QEMU Developers, Mikaël Urankar

[-- Attachment #1: Type: text/plain, Size: 2095 bytes --]

On Fri, Sep 17, 2021 at 9:02 AM Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:

> On 9/17/21 4:56 AM, Warner Losh wrote:
> > From: Mikaël Urankar <mikael.urankar@gmail.com>
> >
> > linux-user/mmap.c: Always zero MAP_ANONYMOUS memory in mmap_frag()
>
> Please use it as subject, "bsd-user/mmap: Always zero MAP_ANONYMOUS
> memory in mmap_frag()"
>
> Then describe:
>
> Similar to the equivalent linux-user commit e6deac9cf99, ...
>

OK. I have three commits like this, so I'll go ahead and edit all three.


> >
> > When mapping MAP_ANONYMOUS memory fragments, still need notice about to
> > set it zero, or it will cause issues.
> >
> > Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
> > Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> > Signed-off-by: Riku Voipio <riku.voipio@linaro.org>
>
> ^ These tags were for another file, not this one, please
> remove them.
>

Gotcha. I wasn't completely sure what to do in this case since they
describe that the work is able to be contributed so I could make a case
either way.

I'll remove them.

Warner


> > [ bsd-user merge by Mikaël Urankar, updated for untagged by Warner Losh ]
> > Signed-off-by: Mikaël Urankar <mikael.urankar@gmail.com>
> > Signed-off-by: Warner Losh <imp@bsdimp.com>
> > ---
> >  bsd-user/mmap.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> > index b40ab9045f..fc3c1480f5 100644
> > --- a/bsd-user/mmap.c
> > +++ b/bsd-user/mmap.c
> > @@ -180,10 +180,12 @@ static int mmap_frag(abi_ulong real_start,
> >          if (prot_new != (prot1 | PROT_WRITE))
> >              mprotect(host_start, qemu_host_page_size, prot_new);
> >      } else {
> > -        /* just update the protection */
> >          if (prot_new != prot1) {
> >              mprotect(host_start, qemu_host_page_size, prot_new);
> >          }
> > +        if (prot_new & PROT_WRITE) {
> > +            memset(g2h_untagged(start), 0, end - start);
> > +        }
> >      }
> >      return 0;
> >  }
> >
>
>

[-- Attachment #2: Type: text/html, Size: 3455 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-09-17 15:13 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-17  2:56 [PATCH 0/9] bsd-user mmap fixes Warner Losh
2021-09-17  2:56 ` [PATCH 1/9] bsd-user: Apply e6deac9cf99 from linux-user (zero anonymous memory) Warner Losh
2021-09-17 15:02   ` Philippe Mathieu-Daudé
2021-09-17 15:10     ` Warner Losh
2021-09-17  2:56 ` [PATCH 2/9] bsd-user: Apply fb7e378cf9c from linux-user (fix FORTIFY warnings) Warner Losh
2021-09-17  2:56 ` [PATCH 3/9] bsd-user: MAP_ symbols are defined, so no need for ifdefs Warner Losh
2021-09-17 15:03   ` Philippe Mathieu-Daudé
2021-09-17  2:56 ` [PATCH 4/9] bsd-user: mmap return ENOMEM on overflow Warner Losh
2021-09-17  2:56 ` [PATCH 5/9] bsd-user: mmap prefer MAP_ANON for BSD Warner Losh
2021-09-17 15:04   ` Philippe Mathieu-Daudé
2021-09-17  2:56 ` [PATCH 6/9] bsd-user: mmap line wrap change Warner Losh
2021-09-17  2:56 ` [PATCH 7/9] bsd-user: Don't try to mmap fd when it is -1 independently from MAP_ANONYMOUS flag Warner Losh
2021-09-17  2:58   ` Warner Losh
2021-09-17  2:56 ` [PATCH 8/9] bsd-user: Implement MAP_EXCL, required by jemalloc in head Warner Losh
2021-09-17  2:56 ` [PATCH 9/9] bsd-user: Apply 86abac06c14 from linux-user (target_mprotect can't fail) Warner Losh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).