* [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* 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 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
* [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* 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
* [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