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

[Helge Deller]

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.ben...@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;

  }