[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] linux-user: Adjust pgd_find_hole_fallback result with guest_
From: |
Alex Bennée |
Subject: |
Re: [PATCH] linux-user: Adjust pgd_find_hole_fallback result with guest_loaddr |
Date: |
Fri, 05 Mar 2021 14:03:43 +0000 |
User-agent: |
mu4e 1.5.8; emacs 28.0.50 |
Ivan A. Melnikov <iv@altlinux.org> writes:
> 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 has non-zero load address.
>
> Failing to take this 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 it is loading
> (e.g. the guest elf interpreter).
>
> This patch also fixes the return type of pgd_find_hole_fallback:
> it returns -1 if no hole is found, so a signed return type
> should be used.
I don't think it should. For one thing the type is preserved as
uintptr_t all the way up the call chain so just changing it here doesn't
help much. -1 is really just a quick way of saying all bits are set
which is the one "fail" value we check for. The address space is big
enough we could theoretically return a chunk of space that otherwise has
the top bit set.
>
> Downstream issue (in Russian): https://bugzilla.altlinux.org/39141
> Signed-off-by: Ivan A. Melnikov <iv@altlinux.org>
> ---
> linux-user/elfload.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index bab4237e90..acd510532c 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2205,9 +2205,11 @@ static void pgb_have_guest_base(const char
> *image_name, abi_ulong guest_loaddr,
> * /proc/self/map. It can potentially take a very long time as we can
> * only dumbly iterate up the host address space seeing if the
> * allocation would work.
> + *
> + * Returns the start addres of the hole found, or -1 if no hole found.
> */
> -static uintptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk,
> - long align, uintptr_t offset)
> +static intptr_t pgd_find_hole_fallback(uintptr_t guest_size, uintptr_t brk,
> + long align, uintptr_t offset)
> {
> uintptr_t base;
>
> @@ -2235,7 +2237,7 @@ static uintptr_t pgd_find_hole_fallback(uintptr_t
> guest_size, uintptr_t brk,
> munmap((void *) align_start, guest_size);
> if (MAP_FIXED_NOREPLACE != 0 ||
> mmap_start == (void *) align_start) {
> - return (uintptr_t) mmap_start + offset;
> + return (intptr_t) mmap_start + offset;
> }
> }
> base += qemu_host_page_size;
> @@ -2259,7 +2261,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 > guest_loaddr) ? (ret - guest_loaddr) : -1;
So I think we just want:
return ret == -1 ? -1 : (ret - guest_loaddr);
do we have a test case that triggers this?
> }
>
> /* The first hole is before the first map entry. */
--
Alex Bennée