From: sisshiki1969 <sissh...@mac.com>

On Fri, Oct 7, 2022 at 9:38 AM Richard Henderson <richard.hender...@linaro.org> 
wrote:
| Although, sorta, this smells like a kernel bug.
| Why should mprotect(-4096, 0, 0) succeed while mprotect(-4096, 4096, 0) fails?

This may be kinda bug compatibility...

| But anyway, if we're going to fix len == 0 to match, we might as well fix all 
3 test
| ordering bugs at the same time.

Yes, I agree, and made another patch.
A validation for wrap-around was added, I think it is neccesory.

A tiny test code was shown below.

```sh
> cat test.c
#include <sys/mman.h>
#include <stdio.h>
#include <stdlib.h>

int main(int argc, char *argv[])
{
  char *addr;
  int prot = PROT_READ | PROT_EXEC;
  int map = MAP_SHARED | MAP_ANONYMOUS;
  addr = mmap(NULL, 4096, prot, map, -1, 0);
  if (addr == 0) {
    perror("mmap");
    exit(EXIT_FAILURE);
  }
  
  void *addrs[] = { (void *)77, NULL, addr };
  for (int i = 0; i < 3; i++) {
    if (mprotect(addrs[i], 0, PROT_READ) == -1) {
      perror("mprotect");
    } else {
      printf("OK\n");
    }
  }

  // invalid prot
  if (mprotect(addr, 2048, PROT_READ | 0x20) == -1) {
    perror("mprotect");
  } else {
    printf("OK\n");
  }
}
> cc test.c -o test
> ./test
mprotect: Invalid argument
OK
OK
mprotect: Invalid argument
> qemu-x86_64 test          # current master
mprotect: Invalid argument
OK
mprotect: Cannot allocate memory
mprotect: Invalid argument
> build/qemu-x86_64 test    # after the patch applied
mprotect: Invalid argument
OK
OK
mprotect: Invalid argument
```

seems good.

Soichiro Isshiki

Signed-off-by: sisshiki1969 <sissh...@mac.com>
---
 linux-user/mmap.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 28f3bc85ed..757709eeba 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -124,17 +124,20 @@ int target_mprotect(abi_ulong start, abi_ulong len, int 
target_prot)
     if ((start & ~TARGET_PAGE_MASK) != 0) {
         return -TARGET_EINVAL;
     }
-    page_flags = validate_prot_to_pageflags(&host_prot, target_prot);
-    if (!page_flags) {
-        return -TARGET_EINVAL;
+    if (len == 0) {
+        return 0;
     }
     len = TARGET_PAGE_ALIGN(len);
     end = start + len;
+    if (end <= start) {
+        return -TARGET_ENOMEM;
+    }
     if (!guest_range_valid_untagged(start, len)) {
         return -TARGET_ENOMEM;
     }
-    if (len == 0) {
-        return 0;
+    page_flags = validate_prot_to_pageflags(&host_prot, target_prot);
+    if (!page_flags) {
+        return -TARGET_EINVAL;
     }
 
     mmap_lock();
-- 
2.25.1


Reply via email to