[PATCH v2 1/4] target/hexagon: fix semihosting OPEN filename handling

[Brian Cain]

HEX_SYS_OPEN copied guest bytes into a fixed-size buffer until it found a NUL.
A missing terminator could overrun that buffer. Use lock_user() with the
ABI filename length and validate its terminating NUL before opening it.

Fixes: 7711fdba88b ("target/hexagon: add main arch-specific semihosting 
operations")
Link: 
https://lore.kernel.org/all/cafeaca9mos6vfhf2uhz8z4cdvab+7dy6hjb2emyz2alse-t...@mail.gmail.com/
Suggested-by: Peter Maydell <peter.mayd...@linaro.org>
Signed-off-by: Brian Cain <brian.c...@oss.qualcomm.com>
---
 target/hexagon/hexswi.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/target/hexagon/hexswi.c b/target/hexagon/hexswi.c
index 4705e915aea..21e5d630a75 100644
--- a/target/hexagon/hexswi.c
+++ b/target/hexagon/hexswi.c
@@ -28,6 +28,7 @@
 #include "semihosting/console.h"
 #include "semihosting/syscalls.h"
 #include "semihosting/guestfd.h"
+#include "semihosting/uaccess.h"
 #include "system/runstate.h"
 
 /* non-arm-compatible semihosting calls */
@@ -461,31 +462,17 @@ static void sim_handle_trap0(CPUHexagonState *env)
 
     case HEX_SYS_OPEN:
     {
-        char filename[BUFSIZ];
+        char *filename;
         target_ulong physical_filename_addr;
         unsigned int filemode;
-        int length;
+        uint32_t filename_len;
+        size_t filename_size;
         int real_openmode;
         int ret, err = 0;
-        int i = 0;
 
         hexagon_read_memory(env, swi_info, 4, &physical_filename_addr, 
retaddr);
         hexagon_read_memory(env, swi_info + 4, 4, &filemode, retaddr);
-        hexagon_read_memory(env, swi_info + 8, 4, &length, retaddr);
-
-        if (length >= BUFSIZ) {
-            qemu_log_mask(LOG_GUEST_ERROR,
-                          "%s: filename too large (%d)\n",
-                          __func__, length);
-            semi_cb(cs, -1, ENAMETOOLONG);
-            break;
-        }
-
-        do {
-            hexagon_read_memory(env, physical_filename_addr + i, 1,
-                                &filename[i], retaddr);
-            i++;
-        } while (filename[i - 1]);
+        hexagon_read_memory(env, swi_info + 8, 4, &filename_len, retaddr);
 
         /* convert ARM ANGEL filemode into host filemode */
         if (filemode < ARRAY_SIZE(angel_to_host_filemode_table)) {
@@ -498,6 +485,16 @@ static void sim_handle_trap0(CPUHexagonState *env)
             break;
         }
 
+        /* The ABI length excludes the filename's terminating NUL. */
+        filename_size = (size_t)filename_len + 1;
+        filename = lock_user(VERIFY_READ, physical_filename_addr,
+                             filename_size, true);
+        if (!filename || filename[filename_len] != '\0') {
+            unlock_user(filename, physical_filename_addr, 0);
+            semi_cb(cs, -1, EFAULT);
+            break;
+        }
+
         if (strcmp(filename, ":tt") == 0 &&
             qemu_semihosting_console_has_chardev()) {
             ret = alloc_guestfd();
@@ -513,6 +510,7 @@ static void sim_handle_trap0(CPUHexagonState *env)
                 ret = guestfd;
             }
         }
+        unlock_user(filename, physical_filename_addr, 0);
         semi_cb(cs, ret, err);
     }
     break;
-- 
2.34.1