[v2] linux-user: Adjust pgd_find_hole_fallback result with guest_loaddr

[PATCH v2] linux-user: Adjust pgd_find_hole_fallback result with guest_loaddr

Posted by Ivan A. Melnikov 3 years, 1 month ago

While pgd_find_hole_fallback returns the beginning of the
hole found, pgb_find_hole returns guest_base, which
is somewhat different as the binary qemu-user is loading
usually have non-zero load address.

Failing to take that into account leads to random crashes
if the hole is "just big enough", but not bigger:
in that case, important mappings (e.g. parts of qemu-user
itself) may be replaced with the binary we are loading
(e.g. guest elf interpreter).

Downstream issue (in Russian): https://bugzilla.altlinux.org/39141
Signed-off-by: Ivan A. Melnikov <iv@altlinux.org>
---
 linux-user/elfload.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index bab4237e90..58281e00f8 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2259,7 +2259,8 @@ static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size,
     brk = (uintptr_t)sbrk(0);
 
     if (!maps) {
-        return pgd_find_hole_fallback(guest_size, brk, align, offset);
+        ret = pgd_find_hole_fallback(guest_size, brk, align, offset);
+        return (ret == (uintptr_t) -1) ? -1 : (ret - guest_loaddr);
     }
 
     /* The first hole is before the first map entry. */
-- 
2.29.2

Re: [PATCH v2] linux-user: Adjust pgd_find_hole_fallback result with guest_loaddr

Posted by Laurent Vivier 3 years, 1 month ago

Le 06/03/2021 à 10:33, Ivan A. Melnikov a écrit :
> While pgd_find_hole_fallback returns the beginning of the
> hole found, pgb_find_hole returns guest_base, which
> is somewhat different as the binary qemu-user is loading
> usually have non-zero load address.
> 
> Failing to take that into account leads to random crashes
> if the hole is "just big enough", but not bigger:
> in that case, important mappings (e.g. parts of qemu-user
> itself) may be replaced with the binary we are loading
> (e.g. guest elf interpreter).
> 
> Downstream issue (in Russian): https://bugzilla.altlinux.org/39141
> Signed-off-by: Ivan A. Melnikov <iv@altlinux.org>
> ---
>  linux-user/elfload.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index bab4237e90..58281e00f8 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2259,7 +2259,8 @@ static uintptr_t pgb_find_hole(uintptr_t guest_loaddr, uintptr_t guest_size,
>      brk = (uintptr_t)sbrk(0);
>  
>      if (!maps) {
> -        return pgd_find_hole_fallback(guest_size, brk, align, offset);
> +        ret = pgd_find_hole_fallback(guest_size, brk, align, offset);
> +        return (ret == (uintptr_t) -1) ? -1 : (ret - guest_loaddr);

You don't want the uintptr_t as ret is intptr_t

>      }
>  
>      /* The first hole is before the first map entry. */
> 

I've already added the patch from Vincent Fazio to fix the problem.

https://github.com/vivier/qemu/commit/5112f50d6e889a2a1098fb2a67a0851641c350f4

It will be in my next pull request.

Thanks,
Laurent