All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.