1. Patchew
  2. QEMU
  3. View series

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

Alex Bennée posted 1 patch 3 weeks, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260903142720.1467316-1-alex.bennee@linaro.org
Maintainers: Laurent Vivier <laurent@vivier.eu>, Helge Deller <deller@gmx.de>, Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>, "Alex Bennée" <alex.bennee@linaro.org>
linux-user/mmap.c                         | 12 ++++++++++++
tests/tcg/multiarch/linux/linux-madvise.c | 20 ++++++++++++++++++++
2 files changed, 32 insertions(+)
[RFC PATCH] linux-user: madvise() on unmapped ranges should return ENOMEM
Posted by Alex Bennée 3 weeks, 2 days ago 
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.
---
 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;
 }
-- 
2.47.3
Re: [RFC PATCH] linux-user: madvise() on unmapped ranges should return ENOMEM
Posted by Helge Deller 2 weeks, 2 days ago 
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;
>   }
Re: [RFC PATCH] linux-user: madvise() on unmapped ranges should return ENOMEM
Posted by Alex Bennée 2 weeks, 2 days ago 
Helge Deller <deller@gmx.de> writes:

> 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?

Yes - ran the test case before and after the changes and it asserts
before.

  ./qemu-aarch64 -strace -d guest_errors,unimp aarch64-linux-user-linux-madvise.test
  681364 uname(0x7f403e54d3c8) = 0
  681364 brk(NULL) = 0x00000000004a7000
  681364 brk(0x00000000004a7b00) = 0x00000000004a7b00
  681364 set_tid_address(0x4a70f0) = 681364
  681364 set_robust_list(0x4a7100,24) = -1 errno=38 (Function not implemented)
  681364 rseq(0x4a77c0,32,0,0xd428bc00)Unsupported syscall: 293
   = -1 errno=38 (Function not implemented)
  681364 prlimit64(0,RLIMIT_STACK,NULL,0x00007f403e54d518) = 0 ({rlim_cur=8388608,rlim_max=-1})
  681364 readlinkat(AT_FDCWD,"/proc/self/exe",0x00007f403e54c4a0,4096) = 73
  681364 getrandom(0x4a6808,8,1) = 8
  681364 brk(NULL) = 0x00000000004a7b00
  681364 brk(0x00000000004c8b00) = 0x00000000004c8b00
  681364 brk(0x00000000004c9000) = 0x00000000004c9000
  681364 mprotect(0x000000000049b000,20480,PROT_READ) = 0
  681364 mmap(NULL,4096,PROT_READ,MAP_PRIVATE|MAP_ANONYMOUS,-1,0) = 0x00007f40405f2000
  681364 mprotect(0x00007f40405f2000,4096,PROT_READ|PROT_WRITE) = 0
  681364 madvise(0x00007f40405f2000,4096,MADV_DONTNEED) = 0
  681364 munmap(0x00007f40405f2000,4096) = 0
  681364 getrandom(0x7f403e54d360,8,1) = 8
  681364 openat(AT_FDCWD,"/tmp/.cmadvisebhAABn",O_RDWR|O_CREAT|O_EXCL,0600) = 3
  681364 unlinkat(AT_FDCWD,"/tmp/.cmadvisebhAABn",0) = 0
  681364 write(3,0x7f403e54d3f7,1) = 1
  681364 ftruncate(3,4096) = 0
  681364 mmap(NULL,4096,PROT_READ,MAP_PRIVATE,3,0) = 0x00007f40405f2000
  681364 mprotect(0x00007f40405f2000,4096,PROT_READ|PROT_WRITE) = 0
  681364 madvise(0x00007f40405f2000,4096,MADV_DONTNEED) = 0
  681364 munmap(0x00007f40405f2000,4096) = 0
  681364 close(3) = 0
  681364 mmap(NULL,4096,PROT_READ,MAP_PRIVATE|MAP_ANONYMOUS,-1,0) = 0x00007f40405f2000
  681364 munmap(0x00007f40405f2000,4096) = 0
  681364 madvise(0x00007f40405f2000,4096,MADV_NORMAL) = 0
  681364 write(2,0x7f403e54cbc8,128)aarch64-linux-user-linux-madvise.test: /home/alex/lsrc/qemu.git/tests/tcg/multiarch/linux/linux-madvise.c:81: test_unmapped: Ass = 128
  681364 write(2,0x7f403e54cbc8,27)ertion `ret == -1' failed.
   = 27
  681364 mmap(NULL,4096,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS,-1,0) = 0x00007f40405f2000
  681364 gettid() = 681364
  681364 getpid() = 681364
  681364 tgkill(681364,681364,SIGIOT) = 0
  --- SIGIOT {si_signo=SIGIOT, si_code=SI_TKILL, si_pid=681364, si_uid=1000} ---
  qemu: uncaught target signal 6 (Aborted) - core dumped
  fish: Job 1, './qemu-aarch64 -strace -d guest…' terminated by signal SIGABRT (Abort)

> I'm asking, because I tried the testcase from the bug report, and
> in qemu I still get 0 (success).

I suspect you've been tripped up by the conversion of tests to meson as
they now have the .test suffix. So with the patch applied and tests
passing:

  cp tests/tcg/aarch64-linux-user-linux-madvise.test .

and then drop back one patch and rebuild and test against the copy of
the test with the test_unmapped support.


>
> 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;
>>   }

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [RFC PATCH] linux-user: madvise() on unmapped ranges should return ENOMEM
Posted by Helge Deller 2 weeks, 2 days ago 
On 9/10/26 12:16, Alex Bennée wrote:
> Helge Deller <deller@gmx.de> writes:
> 
>> 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?
> 
> Yes - ran the test case before and after the changes and it asserts
> before.

Yes, sorry, as mentioned in earlier mail, it was a misconfiguration on my side.

>    ./qemu-aarch64 -strace -d guest_errors,unimp aarch64-linux-user-linux-madvise.test
>    681364 uname(0x7f403e54d3c8) = 0
>    681364 brk(NULL) = 0x00000000004a7000
>    681364 brk(0x00000000004a7b00) = 0x00000000004a7b00
>    681364 set_tid_address(0x4a70f0) = 681364
>    681364 set_robust_list(0x4a7100,24) = -1 errno=38 (Function not implemented)
>    681364 rseq(0x4a77c0,32,0,0xd428bc00)Unsupported syscall: 293
>     = -1 errno=38 (Function not implemented)
>    681364 prlimit64(0,RLIMIT_STACK,NULL,0x00007f403e54d518) = 0 ({rlim_cur=8388608,rlim_max=-1})
>    681364 readlinkat(AT_FDCWD,"/proc/self/exe",0x00007f403e54c4a0,4096) = 73
>    681364 getrandom(0x4a6808,8,1) = 8
>    681364 brk(NULL) = 0x00000000004a7b00
>    681364 brk(0x00000000004c8b00) = 0x00000000004c8b00
>    681364 brk(0x00000000004c9000) = 0x00000000004c9000
>    681364 mprotect(0x000000000049b000,20480,PROT_READ) = 0
>    681364 mmap(NULL,4096,PROT_READ,MAP_PRIVATE|MAP_ANONYMOUS,-1,0) = 0x00007f40405f2000
>    681364 mprotect(0x00007f40405f2000,4096,PROT_READ|PROT_WRITE) = 0
>    681364 madvise(0x00007f40405f2000,4096,MADV_DONTNEED) = 0
>    681364 munmap(0x00007f40405f2000,4096) = 0
>    681364 getrandom(0x7f403e54d360,8,1) = 8
>    681364 openat(AT_FDCWD,"/tmp/.cmadvisebhAABn",O_RDWR|O_CREAT|O_EXCL,0600) = 3
>    681364 unlinkat(AT_FDCWD,"/tmp/.cmadvisebhAABn",0) = 0
>    681364 write(3,0x7f403e54d3f7,1) = 1
>    681364 ftruncate(3,4096) = 0
>    681364 mmap(NULL,4096,PROT_READ,MAP_PRIVATE,3,0) = 0x00007f40405f2000
>    681364 mprotect(0x00007f40405f2000,4096,PROT_READ|PROT_WRITE) = 0
>    681364 madvise(0x00007f40405f2000,4096,MADV_DONTNEED) = 0
>    681364 munmap(0x00007f40405f2000,4096) = 0
>    681364 close(3) = 0
>    681364 mmap(NULL,4096,PROT_READ,MAP_PRIVATE|MAP_ANONYMOUS,-1,0) = 0x00007f40405f2000
>    681364 munmap(0x00007f40405f2000,4096) = 0
>    681364 madvise(0x00007f40405f2000,4096,MADV_NORMAL) = 0
>    681364 write(2,0x7f403e54cbc8,128)aarch64-linux-user-linux-madvise.test: /home/alex/lsrc/qemu.git/tests/tcg/multiarch/linux/linux-madvise.c:81: test_unmapped: Ass = 128
>    681364 write(2,0x7f403e54cbc8,27)ertion `ret == -1' failed.
>     = 27
>    681364 mmap(NULL,4096,PROT_READ|PROT_WRITE,MAP_PRIVATE|MAP_ANONYMOUS,-1,0) = 0x00007f40405f2000
>    681364 gettid() = 681364
>    681364 getpid() = 681364
>    681364 tgkill(681364,681364,SIGIOT) = 0
>    --- SIGIOT {si_signo=SIGIOT, si_code=SI_TKILL, si_pid=681364, si_uid=1000} ---
>    qemu: uncaught target signal 6 (Aborted) - core dumped
>    fish: Job 1, './qemu-aarch64 -strace -d guest…' terminated by signal SIGABRT (Abort)
> 
>> I'm asking, because I tried the testcase from the bug report, and
>> in qemu I still get 0 (success).
> 
> I suspect you've been tripped up by the conversion of tests to meson as
> they now have the .test suffix. 

No, actually I ran the test in a chroot which I set up with binfmt_misc,
and it was configured to use an outdated (v5.something) qemu version :-(

Helge
Re: [RFC PATCH] linux-user: madvise() on unmapped ranges should return ENOMEM
Posted by Helge Deller 2 weeks, 2 days ago 
On 9/9/26 23:17, Helge Deller wrote:
> 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).
Oh, this was a misconfiguration on my side.
The patch works ok!

Reviewed-by: Helge Deller <deller@gmx.de>
Tested-by: Helge Deller <deller@gmx.de>

Thanks!
Helge

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.