[PATCH v3 0/9] bsd-user mmap fixes

[PATCH v3 0/9] bsd-user mmap fixes

[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. Updated to migrate debugging to qemu_log.

v3: reimplement the logging with qemu_log
    redo MAP_EXCL based on review feedback
    update fd == -1 MAP_ANON fix with description of what's going on

v2: do the cherry-picks from linux-user in qemu-style.

Guy Yur (1):
  bsd-user/mmap.c: Don't mmap fd == -1 independently from MAP_ANON flag

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

Mikaël Urankar (2):
  bsd-user/mmap.c: Always zero MAP_ANONYMOUS memory in mmap_frag()
  bsd-user/mmap.c: check pread's return value to fix warnings with
    _FORTIFY_SOURCE

Warner Losh (5):
  bsd-user/mmap.c: MAP_ symbols are defined, so no need for ifdefs
  bsd-user/mmap.c: mmap return ENOMEM on overflow
  bsd-user/mmap.c: mmap prefer MAP_ANON for BSD
  bsd-user/mmap.c: Convert to qemu_log logging for mmap debugging
  bsd-user/mmap.c: assert that target_mprotect cannot fail

 bsd-user/mmap.c | 146 ++++++++++++++++++++++++++----------------------
 1 file changed, 79 insertions(+), 67 deletions(-)

-- 
2.32.0

[PATCH v3 1/9] bsd-user/mmap.c: Always zero MAP_ANONYMOUS memory in mmap_frag()

[Warner Losh]

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

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: Mikaël Urankar <mikael.urankar@gmail.com>
Signed-off-by: Warner Losh <imp@bsdimp.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 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

Re: [PATCH v3 1/9] bsd-user/mmap.c: Always zero MAP_ANONYMOUS memory in mmap_frag()

[Kyle Evans]

On Fri, Oct 8, 2021 at 4:24 PM Warner Losh <imp@bsdimp.com> wrote:
>
> From: Mikaël Urankar <mikael.urankar@gmail.com>
>
> 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: Mikaël Urankar <mikael.urankar@gmail.com>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  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
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

[PATCH v3 2/9] bsd-user/mmap.c: check pread's return value to fix warnings with _FORTIFY_SOURCE

[Warner Losh]

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

Simmilar to the equivalent linux-user: commit fb7e378cf9c, which added
checking to pread's return value. Update to current qemu standards with
{} around the if statement.

Signed-off-by: Mikaël Urankar <mikael.urankar@gmail.com>
Signed-off-by: Warner Losh <imp@bsdimp.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 bsd-user/mmap.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index fc3c1480f5..4f4fa3ab46 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -174,7 +174,9 @@ 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 +595,9 @@ 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

Re: [PATCH v3 2/9] bsd-user/mmap.c: check pread's return value to fix warnings with _FORTIFY_SOURCE

[Kyle Evans]

On Fri, Oct 8, 2021 at 4:24 PM Warner Losh <imp@bsdimp.com> wrote:
>
> From: Mikaël Urankar <mikael.urankar@gmail.com>
>
> Simmilar to the equivalent linux-user: commit fb7e378cf9c, which added
> checking to pread's return value. Update to current qemu standards with
> {} around the if statement.
>
> Signed-off-by: Mikaël Urankar <mikael.urankar@gmail.com>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  bsd-user/mmap.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> index fc3c1480f5..4f4fa3ab46 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -174,7 +174,9 @@ 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 +595,9 @@ 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
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

[PATCH v3 3/9] bsd-user/mmap.c: MAP_ symbols are defined, so no need for ifdefs

[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>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Acked-by: Richard Henderson <richard.henderson@linaro.org>
---
 bsd-user/mmap.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index 4f4fa3ab46..6f33aec58b 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -286,13 +286,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) {
         /*
@@ -407,22 +403,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 ");
         }
@@ -432,11 +424,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
@@ -445,7 +435,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))) {
@@ -453,8 +442,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 */
@@ -462,7 +449,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

Re: [PATCH v3 3/9] bsd-user/mmap.c: MAP_ symbols are defined, so no need for ifdefs

[Kyle Evans]

On Fri, Oct 8, 2021 at 4:24 PM Warner Losh <imp@bsdimp.com> 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>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Acked-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  bsd-user/mmap.c | 14 --------------
>  1 file changed, 14 deletions(-)
>
> diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> index 4f4fa3ab46..6f33aec58b 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -286,13 +286,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) {
>          /*
> @@ -407,22 +403,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 ");
>          }
> @@ -432,11 +424,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
> @@ -445,7 +435,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))) {
> @@ -453,8 +442,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 */
> @@ -462,7 +449,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
>
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

[PATCH v3 4/9] bsd-user/mmap.c: mmap return ENOMEM on overflow

[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>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 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 6f33aec58b..f0be3b12cf 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -455,11 +455,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

Re: [PATCH v3 4/9] bsd-user/mmap.c: mmap return ENOMEM on overflow

[Kyle Evans]

On Fri, Oct 8, 2021 at 4:27 PM Warner Losh <imp@bsdimp.com> wrote:
>
> 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>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  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 6f33aec58b..f0be3b12cf 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -455,11 +455,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
>
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

[PATCH v3 5/9] bsd-user/mmap.c: mmap prefer MAP_ANON for BSD

[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>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 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 f0be3b12cf..301108ed25 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -285,7 +285,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);
     }
@@ -409,7 +409,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) {
@@ -431,7 +431,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;
     }
@@ -533,7 +533,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' */
@@ -696,8 +696,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

Re: [PATCH v3 5/9] bsd-user/mmap.c: mmap prefer MAP_ANON for BSD

[Kyle Evans]

On Fri, Oct 8, 2021 at 4:24 PM Warner Losh <imp@bsdimp.com> 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>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  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 f0be3b12cf..301108ed25 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -285,7 +285,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);
>      }
> @@ -409,7 +409,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) {
> @@ -431,7 +431,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;
>      }
> @@ -533,7 +533,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' */
> @@ -696,8 +696,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
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

[PATCH v3 6/9] bsd-user/mmap.c: Convert to qemu_log logging for mmap debugging

[Warner Losh]

Convert DEBUG_MMAP to qemu_log CPU_LOG_PAGE.

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

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index 301108ed25..face98573f 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -21,8 +21,6 @@
 #include "qemu.h"
 #include "qemu-common.h"
 
-//#define DEBUG_MMAP
-
 static pthread_mutex_t mmap_mutex = PTHREAD_MUTEX_INITIALIZER;
 static __thread int mmap_lock_count;
 
@@ -67,14 +65,11 @@ int target_mprotect(abi_ulong start, abi_ulong len, int prot)
     abi_ulong end, host_start, host_end, addr;
     int prot1, ret;
 
-#ifdef DEBUG_MMAP
-    printf("mprotect: start=0x" TARGET_ABI_FMT_lx
-           "len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c\n", start, len,
-           prot & PROT_READ ? 'r' : '-',
-           prot & PROT_WRITE ? 'w' : '-',
-           prot & PROT_EXEC ? 'x' : '-');
-#endif
-
+    qemu_log_mask(CPU_LOG_PAGE, "mprotect: start=0x" TARGET_ABI_FMT_lx
+                  " len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c\n", start, len,
+                  prot & PROT_READ ? 'r' : '-',
+                  prot & PROT_WRITE ? 'w' : '-',
+                  prot & PROT_EXEC ? 'x' : '-');
     if ((start & ~TARGET_PAGE_MASK) != 0)
         return -EINVAL;
     len = TARGET_PAGE_ALIGN(len);
@@ -391,45 +386,43 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
     abi_ulong ret, end, real_start, real_end, retaddr, host_offset, host_len;
 
     mmap_lock();
-#ifdef DEBUG_MMAP
-    {
-        printf("mmap: start=0x" TARGET_ABI_FMT_lx
-               " len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c flags=",
-               start, len,
-               prot & PROT_READ ? 'r' : '-',
-               prot & PROT_WRITE ? 'w' : '-',
-               prot & PROT_EXEC ? 'x' : '-');
+    if (qemu_loglevel_mask(CPU_LOG_PAGE)) {
+        qemu_log("mmap: start=0x" TARGET_ABI_FMT_lx
+                 " len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c flags=",
+                 start, len,
+                 prot & PROT_READ ? 'r' : '-',
+                 prot & PROT_WRITE ? 'w' : '-',
+                 prot & PROT_EXEC ? 'x' : '-');
         if (flags & MAP_ALIGNMENT_MASK) {
-            printf("MAP_ALIGNED(%u) ", (flags & MAP_ALIGNMENT_MASK)
-                    >> MAP_ALIGNMENT_SHIFT);
+            qemu_log("MAP_ALIGNED(%u) ",
+                     (flags & MAP_ALIGNMENT_MASK) >> MAP_ALIGNMENT_SHIFT);
         }
         if (flags & MAP_GUARD) {
-            printf("MAP_GUARD ");
+            qemu_log("MAP_GUARD ");
         }
         if (flags & MAP_FIXED) {
-            printf("MAP_FIXED ");
+            qemu_log("MAP_FIXED ");
         }
         if (flags & MAP_ANON) {
-            printf("MAP_ANON ");
+            qemu_log("MAP_ANON ");
         }
         if (flags & MAP_EXCL) {
-            printf("MAP_EXCL ");
+            qemu_log("MAP_EXCL ");
         }
         if (flags & MAP_PRIVATE) {
-            printf("MAP_PRIVATE ");
+            qemu_log("MAP_PRIVATE ");
         }
         if (flags & MAP_SHARED) {
-            printf("MAP_SHARED ");
+            qemu_log("MAP_SHARED ");
         }
         if (flags & MAP_NOCORE) {
-            printf("MAP_NOCORE ");
+            qemu_log("MAP_NOCORE ");
         }
         if (flags & MAP_STACK) {
-            printf("MAP_STACK ");
+            qemu_log("MAP_STACK ");
         }
-        printf("fd=%d offset=0x%llx\n", fd, offset);
+        qemu_log("fd=%d offset=0x%lx\n", fd, offset);
     }
-#endif
 
     if ((flags & MAP_ANON) && fd != -1) {
         errno = EINVAL;
-- 
2.32.0

Re: [PATCH v3 6/9] bsd-user/mmap.c: Convert to qemu_log logging for mmap debugging

[Richard Henderson]

On 10/8/21 2:23 PM, Warner Losh wrote:
> Convert DEBUG_MMAP to qemu_log CPU_LOG_PAGE.
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/mmap.c | 53 +++++++++++++++++++++----------------------------
>   1 file changed, 23 insertions(+), 30 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v3 6/9] bsd-user/mmap.c: Convert to qemu_log logging for mmap debugging

[Philippe Mathieu-Daudé]

On 10/8/21 23:23, Warner Losh wrote:
> Convert DEBUG_MMAP to qemu_log CPU_LOG_PAGE.
> 
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/mmap.c | 53 +++++++++++++++++++++----------------------------
>  1 file changed, 23 insertions(+), 30 deletions(-)

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

Re: [PATCH v3 6/9] bsd-user/mmap.c: Convert to qemu_log logging for mmap debugging

[Kyle Evans]

On Fri, Oct 8, 2021 at 4:25 PM Warner Losh <imp@bsdimp.com> wrote:
>
> Convert DEBUG_MMAP to qemu_log CPU_LOG_PAGE.
>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/mmap.c | 53 +++++++++++++++++++++----------------------------
>  1 file changed, 23 insertions(+), 30 deletions(-)
>
> diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> index 301108ed25..face98573f 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -21,8 +21,6 @@
>  #include "qemu.h"
>  #include "qemu-common.h"
>
> -//#define DEBUG_MMAP
> -
>  static pthread_mutex_t mmap_mutex = PTHREAD_MUTEX_INITIALIZER;
>  static __thread int mmap_lock_count;
>
> @@ -67,14 +65,11 @@ int target_mprotect(abi_ulong start, abi_ulong len, int prot)
>      abi_ulong end, host_start, host_end, addr;
>      int prot1, ret;
>
> -#ifdef DEBUG_MMAP
> -    printf("mprotect: start=0x" TARGET_ABI_FMT_lx
> -           "len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c\n", start, len,
> -           prot & PROT_READ ? 'r' : '-',
> -           prot & PROT_WRITE ? 'w' : '-',
> -           prot & PROT_EXEC ? 'x' : '-');
> -#endif
> -
> +    qemu_log_mask(CPU_LOG_PAGE, "mprotect: start=0x" TARGET_ABI_FMT_lx
> +                  " len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c\n", start, len,
> +                  prot & PROT_READ ? 'r' : '-',
> +                  prot & PROT_WRITE ? 'w' : '-',
> +                  prot & PROT_EXEC ? 'x' : '-');
>      if ((start & ~TARGET_PAGE_MASK) != 0)
>          return -EINVAL;
>      len = TARGET_PAGE_ALIGN(len);
> @@ -391,45 +386,43 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
>      abi_ulong ret, end, real_start, real_end, retaddr, host_offset, host_len;
>
>      mmap_lock();
> -#ifdef DEBUG_MMAP
> -    {
> -        printf("mmap: start=0x" TARGET_ABI_FMT_lx
> -               " len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c flags=",
> -               start, len,
> -               prot & PROT_READ ? 'r' : '-',
> -               prot & PROT_WRITE ? 'w' : '-',
> -               prot & PROT_EXEC ? 'x' : '-');
> +    if (qemu_loglevel_mask(CPU_LOG_PAGE)) {
> +        qemu_log("mmap: start=0x" TARGET_ABI_FMT_lx
> +                 " len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c flags=",
> +                 start, len,
> +                 prot & PROT_READ ? 'r' : '-',
> +                 prot & PROT_WRITE ? 'w' : '-',
> +                 prot & PROT_EXEC ? 'x' : '-');
>          if (flags & MAP_ALIGNMENT_MASK) {
> -            printf("MAP_ALIGNED(%u) ", (flags & MAP_ALIGNMENT_MASK)
> -                    >> MAP_ALIGNMENT_SHIFT);
> +            qemu_log("MAP_ALIGNED(%u) ",
> +                     (flags & MAP_ALIGNMENT_MASK) >> MAP_ALIGNMENT_SHIFT);
>          }
>          if (flags & MAP_GUARD) {
> -            printf("MAP_GUARD ");
> +            qemu_log("MAP_GUARD ");
>          }
>          if (flags & MAP_FIXED) {
> -            printf("MAP_FIXED ");
> +            qemu_log("MAP_FIXED ");
>          }
>          if (flags & MAP_ANON) {
> -            printf("MAP_ANON ");
> +            qemu_log("MAP_ANON ");
>          }
>          if (flags & MAP_EXCL) {
> -            printf("MAP_EXCL ");
> +            qemu_log("MAP_EXCL ");
>          }
>          if (flags & MAP_PRIVATE) {
> -            printf("MAP_PRIVATE ");
> +            qemu_log("MAP_PRIVATE ");
>          }
>          if (flags & MAP_SHARED) {
> -            printf("MAP_SHARED ");
> +            qemu_log("MAP_SHARED ");
>          }
>          if (flags & MAP_NOCORE) {
> -            printf("MAP_NOCORE ");
> +            qemu_log("MAP_NOCORE ");
>          }
>          if (flags & MAP_STACK) {
> -            printf("MAP_STACK ");
> +            qemu_log("MAP_STACK ");
>          }
> -        printf("fd=%d offset=0x%llx\n", fd, offset);
> +        qemu_log("fd=%d offset=0x%lx\n", fd, offset);
>      }
> -#endif
>
>      if ((flags & MAP_ANON) && fd != -1) {
>          errno = EINVAL;
> --
> 2.32.0
>
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

[PATCH v3 7/9] bsd-user/mmap.c: Don't mmap fd == -1 independently from MAP_ANON flag

[Warner Losh]

From: Guy Yur <guyyur@gmail.com>

Switch checks for !(flags & MAP_ANONYMOUS) with checks for fd != -1.
MAP_STACK and MAP_GUARD both require fd == -1 and don't require mapping
the fd either. Add analysis from Guy Yur detailing the different cases
for MAP_GUARD and MAP_STACK.

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 | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index face98573f..4ecd949a10 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -127,7 +127,27 @@ error:
     return ret;
 }
 
-/* map an incomplete host page */
+/*
+ * map an incomplete host page
+ *
+ * mmap_frag can be called with a valid fd, if flags doesn't contain one of
+ * MAP_ANON, MAP_STACK, MAP_GUARD. If we need to map a page in those cases, we
+ * pass fd == -1. However, if flags contains MAP_GUARD then MAP_ANON cannot be
+ * added.
+ *
+ * * If fd is valid (not -1) we want to map the pages with MAP_ANON.
+ * * If flags contains MAP_GUARD we don't want to add MAP_ANON because it
+ *   will be rejected.  See kern_mmap's enforcing of constraints for MAP_GUARD
+ *   in sys/vm/vm_mmap.c.
+ * * If flags contains MAP_ANON it doesn't matter if we add it or not.
+ * * If flags contains MAP_STACK, mmap adds MAP_ANON when called so doesn't
+ *   matter if we add it or not either. See enforcing of constraints for
+ *   MAP_STACK in kern_mmap.
+ *
+ * Don't add MAP_ANON for the flags that use fd == -1 without specifying the
+ * flags directly, with the assumption that future flags that require fd == -1
+ * will also not require MAP_ANON.
+ */
 static int mmap_frag(abi_ulong real_start,
                      abi_ulong start, abi_ulong end,
                      int prot, int flags, int fd, abi_ulong offset)
@@ -147,9 +167,9 @@ static int mmap_frag(abi_ulong real_start,
     }
 
     if (prot1 == 0) {
-        /* no page was there, so we allocate one */
+        /* no page was there, so we allocate one. See also above. */
         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;
@@ -157,7 +177,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 &&
@@ -565,7 +585,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

Re: [PATCH v3 7/9] bsd-user/mmap.c: Don't mmap fd == -1 independently from MAP_ANON flag

[Richard Henderson]

On 10/8/21 2:23 PM, Warner Losh wrote:
> From: Guy Yur<guyyur@gmail.com>
> 
> Switch checks for !(flags & MAP_ANONYMOUS) with checks for fd != -1.
> MAP_STACK and MAP_GUARD both require fd == -1 and don't require mapping
> the fd either. Add analysis from Guy Yur detailing the different cases
> for MAP_GUARD and MAP_STACK.
> 
> 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 | 30 +++++++++++++++++++++++++-----
>   1 file changed, 25 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v3 7/9] bsd-user/mmap.c: Don't mmap fd == -1 independently from MAP_ANON flag

[Kyle Evans]

On Fri, Oct 8, 2021 at 4:29 PM Warner Losh <imp@bsdimp.com> wrote:
>
> From: Guy Yur <guyyur@gmail.com>
>
> Switch checks for !(flags & MAP_ANONYMOUS) with checks for fd != -1.
> MAP_STACK and MAP_GUARD both require fd == -1 and don't require mapping
> the fd either. Add analysis from Guy Yur detailing the different cases
> for MAP_GUARD and MAP_STACK.
>
> 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 | 30 +++++++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> index face98573f..4ecd949a10 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -127,7 +127,27 @@ error:
>      return ret;
>  }
>
> -/* map an incomplete host page */
> +/*
> + * map an incomplete host page
> + *
> + * mmap_frag can be called with a valid fd, if flags doesn't contain one of
> + * MAP_ANON, MAP_STACK, MAP_GUARD. If we need to map a page in those cases, we
> + * pass fd == -1. However, if flags contains MAP_GUARD then MAP_ANON cannot be
> + * added.
> + *
> + * * If fd is valid (not -1) we want to map the pages with MAP_ANON.
> + * * If flags contains MAP_GUARD we don't want to add MAP_ANON because it
> + *   will be rejected.  See kern_mmap's enforcing of constraints for MAP_GUARD
> + *   in sys/vm/vm_mmap.c.
> + * * If flags contains MAP_ANON it doesn't matter if we add it or not.
> + * * If flags contains MAP_STACK, mmap adds MAP_ANON when called so doesn't
> + *   matter if we add it or not either. See enforcing of constraints for
> + *   MAP_STACK in kern_mmap.
> + *
> + * Don't add MAP_ANON for the flags that use fd == -1 without specifying the
> + * flags directly, with the assumption that future flags that require fd == -1
> + * will also not require MAP_ANON.
> + */
>  static int mmap_frag(abi_ulong real_start,
>                       abi_ulong start, abi_ulong end,
>                       int prot, int flags, int fd, abi_ulong offset)
> @@ -147,9 +167,9 @@ static int mmap_frag(abi_ulong real_start,
>      }
>
>      if (prot1 == 0) {
> -        /* no page was there, so we allocate one */
> +        /* no page was there, so we allocate one. See also above. */
>          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;
> @@ -157,7 +177,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 &&
> @@ -565,7 +585,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
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

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

[Warner Losh]

From: Kyle Evans <kevans@FreeBSD.org>

jemalloc requires a working MAP_EXCL. Ensure that no page is double
mapped when specified. In addition, use guest_range_valid_untagged to
test for valid ranges of pages rather than an incomplete inlined version
of the test that might be wrong.

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

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index 4ecd949a10..066d9c10ff 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -403,7 +403,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();
     if (qemu_loglevel_mask(CPU_LOG_PAGE)) {
@@ -574,12 +574,10 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
          * 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) {
+        if (!guest_range_valid_untagged(start, len)) {
             errno = EINVAL;
             goto fail;
         }
-#endif
 
         /*
          * worst case: we cannot map the file because the offset is not
@@ -614,6 +612,12 @@ 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) && page_check_range(start, len, 0) < 0) {
+            errno = EINVAL;
+            goto fail;
+        }
+
         /* handle the start of the mapping */
         if (start > real_start) {
             if (real_end == real_start + qemu_host_page_size) {
-- 
2.32.0

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

[Richard Henderson]

On 10/8/21 2:23 PM, Warner Losh wrote:
> From: Kyle Evans<kevans@FreeBSD.org>
> 
> jemalloc requires a working MAP_EXCL. Ensure that no page is double
> mapped when specified. In addition, use guest_range_valid_untagged to
> test for valid ranges of pages rather than an incomplete inlined version
> of the test that might be wrong.
> 
> Signed-off-by: Kyle Evans<kevans@FreeBSD.org>
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/mmap.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH v3 9/9] bsd-user/mmap.c: assert that target_mprotect cannot fail

[Warner Losh]

Similar to the equivalent linux-user change 86abac06c14. 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: Mikaël Urankar <mikael.urankar@gmail.com>
Signed-off-by: Warner Losh <imp@bsdimp.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 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 066d9c10ff..4586ad27d0 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -604,10 +604,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
             }
             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

Re: [PATCH v3 9/9] bsd-user/mmap.c: assert that target_mprotect cannot fail

[Kyle Evans]

On Fri, Oct 8, 2021 at 4:31 PM Warner Losh <imp@bsdimp.com> wrote:
>
> Similar to the equivalent linux-user change 86abac06c14. 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: Mikaël Urankar <mikael.urankar@gmail.com>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  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 066d9c10ff..4586ad27d0 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -604,10 +604,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
>              }
>              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
>
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>