On 2023/08/08 22:48, Alex Bennée wrote:
Akihiko Odaki <akihiko.od...@daynix.com> writes:
On 2023/08/08 18:43, Alex Bennée wrote:
Richard Henderson <richard.hender...@linaro.org> writes:
Use this as extra protection for the guest mapping over
any qemu host mappings.
Tested-by: Helge Deller <del...@gmx.de>
Reviewed-by: Helge Deller <del...@gmx.de>
Reviewed-by: Akihiko Odaki <akihiko.od...@daynix.com>
Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
---
linux-user/elfload.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 36e4026f05..1b4bb2d5af 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -3147,8 +3147,11 @@ static void load_elf_image(const char *image_name, int
image_fd,
/*
* Reserve address space for all of this.
*
- * In the case of ET_EXEC, we supply MAP_FIXED so that we get
- * exactly the address range that is required.
+ * In the case of ET_EXEC, we supply MAP_FIXED_NOREPLACE so that we get
+ * exactly the address range that is required. Without reserved_va,
+ * the guest address space is not isolated. We have attempted to avoid
+ * conflict with the host program itself via probe_guest_base, but using
+ * MAP_FIXED_NOREPLACE instead of MAP_FIXED provides an extra check.
*
* Otherwise this is ET_DYN, and we are searching for a location
* that can hold the memory space required. If the image is
@@ -3160,7 +3163,7 @@ static void load_elf_image(const char *image_name, int
image_fd,
*/
load_addr = target_mmap(loaddr, (size_t)hiaddr - loaddr + 1, PROT_NONE,
MAP_PRIVATE | MAP_ANON | MAP_NORESERVE |
- (ehdr->e_type == ET_EXEC ? MAP_FIXED : 0),
+ (ehdr->e_type == ET_EXEC ? MAP_FIXED_NOREPLACE :
0),
-1, 0);
We should probably also check the result == load_addr for the places
where MAP_FIXED_NOREPLACE isn't supported as we have this in osdep.h:
#ifndef MAP_FIXED_NOREPLACE
#define MAP_FIXED_NOREPLACE 0
#endif
See 2667e069e7 (linux-user: don't use MAP_FIXED in
pgd_find_hole_fallback)
It assumes target_mmap() emulates MAP_FIXED_NOREPLACE when the host
does not support it as commit e69e032d1a ("linux-user: Use
MAP_FIXED_NOREPLACE for do_brk()") already does, but defining
MAP_FIXED_NOREPLACE zero breaks such emulation. I wrote a fix:
https://patchew.org/QEMU/20230808115242.73025-1-akihiko.od...@daynix.com/
Hmm doesn't that push the problem to real mmap() calls to a host system
that doesn't support MAP_FIXED_NOREPLACE?
That can happen even without that patch if you run QEMU built with a new
libc on old Linux so we should have prepared for that kind of situation.
The man page also says:
> Note that older kernels which do not recognize the MAP_FIXED_NOREPLACE
> flag will typically (upon detecting a collision with a preexisting
> mapping) fall back to a “non-MAP_FIXED” type of behavior: they will
> return an address that is different from the requested address.
> Therefore, backward-compatible software should check the returned
> address against the requested address.
https://man7.org/linux/man-pages/man2/mmap.2.html
It basically means MAP_FIXED_NOREPLACE has no effect on a host that
doesn't support it, and the existing code checking the returned address
should continue to work.
I wonder if we need an internal flag rather than overloading the host
flags?
if (load_addr == -1) {
goto exit_mmap;
