[16/17] bsd-user: Define validate_prot_to_pageflags and use in mprotect

Commit Message

Warner Losh Aug. 2, 2024, 11:56 p.m. UTC

Define validate_prot_to_pageflags. Use it in target_mprotect to validate
the flags. Our taraget_mmap needs more work before it can be used there,
do don't copy linux-user's use of it there. This should hvae no net
functional change, but does make target_mprotect more similar to
linux-user's.

Signed-off-by: Warner Losh <[e-mail]>
---
 bsd-user/mmap.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

Comments

Richard Henderson Aug. 4, 2024, 9:44 p.m. UTC | #1

On 8/3/24 09:56, Warner Losh wrote:
> Define validate_prot_to_pageflags. Use it in target_mprotect to validate
> the flags. Our taraget_mmap needs more work before it can be used there,
> do don't copy linux-user's use of it there. This should hvae no net
> functional change, but does make target_mprotect more similar to
> linux-user's.
> 
> Signed-off-by: Warner Losh <[e-mail]>
> ---
>   bsd-user/mmap.c | 28 +++++++++++++++++++++++-----
>   1 file changed, 23 insertions(+), 5 deletions(-)

> +/*
> + * Validate target prot bitmask.
> + * Return the prot bitmask for the host in *HOST_PROT.
> + * Return 0 if the target prot bitmask is invalid, otherwise
> + * the internal qemu page_flags (which will include PAGE_VALID).
> + */
> +static int validate_prot_to_pageflags(int prot)
> +{
> +    int valid = PROT_READ | PROT_WRITE | PROT_EXEC;
> +    int page_flags = (prot & PAGE_RWX) | PAGE_VALID;
> +
> +    return prot & ~valid ? 0 : page_flags;
> +}

Comment still refers to @host_prot, which you removed.

Otherwise,
Reviewed-by: Richard Henderson <[e-mail]>

r~

Patch

diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index ffecf52a72a..3c48a188e88 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -60,12 +60,26 @@  void mmap_fork_end(int child)
         pthread_mutex_unlock(&mmap_mutex);
 }
 
+/*
+ * Validate target prot bitmask.
+ * Return the prot bitmask for the host in *HOST_PROT.
+ * Return 0 if the target prot bitmask is invalid, otherwise
+ * the internal qemu page_flags (which will include PAGE_VALID).
+ */
+static int validate_prot_to_pageflags(int prot)
+{
+    int valid = PROT_READ | PROT_WRITE | PROT_EXEC;
+    int page_flags = (prot & PAGE_RWX) | PAGE_VALID;
+
+    return prot & ~valid ? 0 : page_flags;
+}
+
 /* NOTE: all the constants are the HOST ones, but addresses are target. */
 int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
 {
     int host_page_size = qemu_real_host_page_size();
     abi_ulong end, host_start, host_end, addr;
-    int prot1, ret;
+    int prot1, ret, page_flags;
 
     qemu_log_mask(CPU_LOG_PAGE, "mprotect: start=0x" TARGET_ABI_FMT_lx
                   " len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c\n", start, len,
@@ -74,14 +88,18 @@  int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
                   target_prot & PROT_EXEC ? 'x' : '-');
     if ((start & ~TARGET_PAGE_MASK) != 0)
         return -EINVAL;
+    page_flags = validate_prot_to_pageflags(target_prot);
+    if (!page_flags) {
+        return -TARGET_EINVAL;
+    }
     len = TARGET_PAGE_ALIGN(len);
+    if (len == 0)
+        return 0;
     if (!guest_range_valid_untagged(start, len)) {
         return -ENOMEM;
     }
-    end = start + len;
     target_prot &= PROT_READ | PROT_WRITE | PROT_EXEC;
-    if (len == 0)
-        return 0;
+    end = start + len;
 
     mmap_lock();
     host_start = start & -host_page_size;
@@ -122,7 +140,7 @@  int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
         if (ret != 0)
             goto error;
     }
-    page_set_flags(start, start + len - 1, target_prot | PAGE_VALID);
+    page_set_flags(start, start + len - 1, page_flags);
     mmap_unlock();
     return 0;
 error: