[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-stable] [PATCH v3 05/11] linux-user: fix mmap/munmap/mprotect/
From: |
Laurent Vivier |
Subject: |
Re: [Qemu-stable] [PATCH v3 05/11] linux-user: fix mmap/munmap/mprotect/mremap/shmat |
Date: |
Tue, 6 Mar 2018 13:02:59 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 |
Le 01/03/2018 à 18:36, Max Filippov a écrit :
> In linux-user QEMU that runs for a target with TARGET_ABI_BITS bigger
> than L1_MAP_ADDR_SPACE_BITS an assertion in page_set_flags fires when
> mmap, munmap, mprotect, mremap or shmat is called for an address outside
> the guest address space. mmap and mprotect should return ENOMEM in such
> case.
>
> Introduce macro guest_range_valid that verifies if address range is
> within guest address space and does not wrap around. Use that macro in
> mmap/munmap/mprotect/mremap/shmat for error checking.
>
> Cc: address@hidden
> Cc: Riku Voipio <address@hidden>
> Cc: Laurent Vivier <address@hidden>
> Signed-off-by: Max Filippov <address@hidden>
> ---
> Changes v2->v3:
> - fix comparison in guest_valid: it must be 'less' to preserve the existing
> functionality, not 'less or equal'.
> - fix guest_range_valid: it may not use guest_valid, because single range
> that occupies all of the guest address space is valid.
>
> These changes fix assertion in page_check_range called from open_self_maps.
>
> include/exec/cpu-all.h | 2 +-
> include/exec/cpu_ldst.h | 12 +++++++-----
> linux-user/mmap.c | 20 +++++++++++++++-----
> linux-user/syscall.c | 3 +++
> 4 files changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index 0b141683f095..12bd049997ac 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -160,7 +160,7 @@ extern int have_guest_base;
> extern unsigned long reserved_va;
>
> #define GUEST_ADDR_MAX (reserved_va ? reserved_va : \
> - (1ul << TARGET_VIRT_ADDR_SPACE_BITS) - 1)
> + (2ul << (TARGET_VIRT_ADDR_SPACE_BITS - 1)) - 1)
I don't understand why you do this change. Could you explain?
> #else
>
> #include "exec/hwaddr.h"
> diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
> index 191f2e962a3c..6528fd829ffe 100644
> --- a/include/exec/cpu_ldst.h
> +++ b/include/exec/cpu_ldst.h
> @@ -53,14 +53,16 @@
>
> #if HOST_LONG_BITS <= TARGET_VIRT_ADDR_SPACE_BITS
> #define h2g_valid(x) 1
> +#define guest_valid(x) 1
> #else
> -#define h2g_valid(x) ({ \
> - unsigned long __guest = (unsigned long)(x) - guest_base; \
> - (__guest < (1ul << TARGET_VIRT_ADDR_SPACE_BITS)) && \
> - (!reserved_va || (__guest < reserved_va)); \
> -})
> +#define h2g_valid(x) guest_valid((unsigned long)(x) - guest_base)
> +#define guest_valid(x) ((x) < GUEST_ADDR_MAX)
> #endif
I think you can just define h2g_valid(x) after the "endif":
#define h2g_valid(x) guest_valid((unsigned long)(x) - guest_base)
it will be set according to the guest_valid() definition ("1" or the
result of the evaluation).
Thanks,
Laurent