Re: [PATCH v3 17/33] linux-user: Move some mmap checks outside the lock

[Ilya Leoshkevich]

On Tue, Jan 02, 2024 at 12:57:52PM +1100, Richard Henderson wrote:
> Basic validation of operands does not require the lock.
> Hoist them from target_mmap__locked back into target_mmap.
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/mmap.c | 107 +++++++++++++++++++++++-----------------------
>  1 file changed, 53 insertions(+), 54 deletions(-)
> 
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index b4c3cc65aa..fbaea832c5 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c

[...]

> @@ -778,13 +726,64 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, 
> int target_prot,
>                       int flags, int fd, off_t offset)
>  {
>      abi_long ret;
> +    int page_flags;
>  
>      trace_target_mmap(start, len, target_prot, flags, fd, offset);
> +
> +    if (!len) {
> +        errno = EINVAL;
> +        return -1;
> +    }
> +
> +    page_flags = validate_prot_to_pageflags(target_prot);
> +    if (!page_flags) {
> +        errno = EINVAL;
> +        return -1;
> +    }
> +
> +    /* Also check for overflows... */
> +    len = TARGET_PAGE_ALIGN(len);
> +    if (!len || len != (size_t)len) {
> +        errno = ENOMEM;
> +        return -1;
> +    }

The overflow fix is probably worth mentioning in the commit message (or
even deserves a separate commit, for backporting into stable).
Regardless:

Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>