Re: [RFC PATCH] linux-user: madvise() on unmapped ranges should return ENOMEM

[Helge Deller <deller@gmx.de>]

Hi Alex,

On 9/3/26 16:27, Alex Bennée wrote:
> Per madvise(2) and the Linux kernel implementation (madvise_walk_vmas),
> madvise() must validate that the requested range is currently mapped
> and return -ENOMEM if any page in the range is unmapped.
> 
> Add a page_check_range(start, len, PAGE_VALID) check for valid advice
> values before proceeding with the advice actions. In addition, extend the
> tcg multiarch test linux-madvise.c to test this behaviour.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4382
> AI-used-for: importing and validating test case
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
> NOTE
>    - again testing the minimal agents which did stop and say:
> 
> *(Note: Per user policy, git commits are never executed automatically by the agent. Please review the diff with `git diff` and commit the changes if you are satisfied.)*
> 
>    but non-the-less imported the test and wrote a crap patch which I
>    have re-done dropping a load of unneeded verbosity.


Did you test this patch?
If yes, did it work for you?
I'm asking, because I tried the testcase from the bug report, and
in qemu I still get 0 (success).

Helge

> ---
>   linux-user/mmap.c                         | 12 ++++++++++++
>   tests/tcg/multiarch/linux/linux-madvise.c | 20 ++++++++++++++++++++
>   2 files changed, 32 insertions(+)
> 
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index cc0c2ee6c27..4066072ff45 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -1307,6 +1307,16 @@ abi_long target_madvise(abi_ulong start, abi_ulong len_in, int advice)
>        * though.
>        */
>       mmap_lock();
> +
> +    /*
> +     * Whatever advice if the pages are not currently mapped, or are
> +     * outside the address space of the process.
> +     */
> +    if (!page_check_range(start, len, PAGE_VALID)) {
> +        ret = -TARGET_ENOMEM;
> +        goto unlock;
> +    }
> +
>       switch (advice) {
>       case MADV_NORMAL:
>       case MADV_RANDOM:
> @@ -1358,6 +1368,8 @@ abi_long target_madvise(abi_ulong start, abi_ulong len_in, int advice)
>           ret = -EINVAL; /* not yet known advise */
>           break;
>       }
> +
> + unlock:
>       mmap_unlock();
>   
>       return ret;
> diff --git a/tests/tcg/multiarch/linux/linux-madvise.c b/tests/tcg/multiarch/linux/linux-madvise.c
> index 539fb3b7726..ebb9666c919 100644
> --- a/tests/tcg/multiarch/linux/linux-madvise.c
> +++ b/tests/tcg/multiarch/linux/linux-madvise.c
> @@ -1,4 +1,5 @@
>   #include <assert.h>
> +#include <errno.h>
>   #include <stdlib.h>
>   #include <sys/mman.h>
>   #include <unistd.h>
> @@ -63,10 +64,29 @@ static void test_file(void)
>       assert(ret == 0);
>   }
>   
> +static void test_unmapped(void)
> +{
> +    int pagesize = getpagesize();
> +    void *page;
> +    int ret;
> +
> +    page = mmap(NULL, pagesize, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> +    assert(page != MAP_FAILED);
> +
> +    ret = munmap(page, pagesize);
> +    assert(ret == 0);
> +
> +    errno = 0;
> +    ret = madvise(page, pagesize, MADV_NORMAL);
> +    assert(ret == -1);
> +    assert(errno == ENOMEM);
> +}
> +
>   int main(void)
>   {
>       test_anonymous();
>       test_file();
> +    test_unmapped();
>   
>       return EXIT_SUCCESS;
>   }