qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: qemu-devel@nongnu.org
Cc: Blue Swirl <blauwirbel@gmail.com>,
	Meador Inge <meadori@codesourcery.com>,
	Paul Brook <paul@codesourcery.com>,
	patches@linaro.org
Subject: [Qemu-devel] [PATCH 2/3] target-m68k/m68k-semi: Handle get_user failure
Date: Mon, 29 Oct 2012 12:05:10 +0000	[thread overview]
Message-ID: <1351512311-19106-3-git-send-email-peter.maydell@linaro.org> (raw)
In-Reply-To: <1351512311-19106-1-git-send-email-peter.maydell@linaro.org>

Handle failure of get_user accessing the semihosting
argument block, rather than simply ignoring the failures.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-m68k/m68k-semi.c | 144 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 93 insertions(+), 51 deletions(-)

diff --git a/target-m68k/m68k-semi.c b/target-m68k/m68k-semi.c
index fed44ea..d569bf1 100644
--- a/target-m68k/m68k-semi.c
+++ b/target-m68k/m68k-semi.c
@@ -153,17 +153,21 @@ static void m68k_semi_cb(CPUM68KState *env, target_ulong ret, target_ulong err)
     put_user_u32(err, args + 4);
 }
 
-#define ARG(n)					\
-({						\
-    target_ulong __arg;				\
-    /* FIXME - handle get_user() failure */	\
-    get_user_ual(__arg, args + (n) * 4);	\
-    __arg;					\
-})
-#define PARG(x) ((unsigned long)ARG(x))
+/* Read the input value from the argument block; fail the semihosting
+ * call if the memory read fails.
+ */
+#define GET_ARG(n) do {                                 \
+    if (get_user_ual(arg ## n, args + (n) * 4)) {       \
+        result = -1;                                    \
+        errno = EFAULT;                                 \
+        goto failed;                                    \
+    }                                                   \
+} while (0)
+
 void do_m68k_semihosting(CPUM68KState *env, int nr)
 {
     uint32_t args;
+    target_ulong arg0, arg1, arg2, arg3;
     void *p;
     void *q;
     uint32_t len;
@@ -175,27 +179,33 @@ void do_m68k_semihosting(CPUM68KState *env, int nr)
         gdb_exit(env, env->dregs[0]);
         exit(env->dregs[0]);
     case HOSTED_OPEN:
+        GET_ARG(0);
+        GET_ARG(1);
+        GET_ARG(2);
+        GET_ARG(3);
         if (use_gdb_syscalls()) {
-            gdb_do_syscall(m68k_semi_cb, "open,%s,%x,%x", ARG(0), (int)ARG(1),
-                           ARG(2), ARG(3));
+            gdb_do_syscall(m68k_semi_cb, "open,%s,%x,%x", arg0, (int)arg1,
+                           arg2, arg3);
             return;
         } else {
-            if (!(p = lock_user_string(ARG(0)))) {
+            p = lock_user_string(arg0);
+            if (!p) {
                 /* FIXME - check error code? */
                 result = -1;
             } else {
-                result = open(p, translate_openflags(ARG(2)), ARG(3));
-                unlock_user(p, ARG(0), 0);
+                result = open(p, translate_openflags(arg2), arg3);
+                unlock_user(p, arg0, 0);
             }
         }
         break;
     case HOSTED_CLOSE:
         {
             /* Ignore attempts to close stdin/out/err.  */
-            int fd = ARG(0);
+            GET_ARG(0);
+            int fd = arg0;
             if (fd > 2) {
                 if (use_gdb_syscalls()) {
-                    gdb_do_syscall(m68k_semi_cb, "close,%x", ARG(0));
+                    gdb_do_syscall(m68k_semi_cb, "close,%x", arg0);
                     return;
                 } else {
                     result = close(fd);
@@ -206,47 +216,59 @@ void do_m68k_semihosting(CPUM68KState *env, int nr)
             break;
         }
     case HOSTED_READ:
-        len = ARG(2);
+        GET_ARG(0);
+        GET_ARG(1);
+        GET_ARG(2);
+        len = arg2;
         if (use_gdb_syscalls()) {
             gdb_do_syscall(m68k_semi_cb, "read,%x,%x,%x",
-                           ARG(0), ARG(1), len);
+                           arg0, arg1, len);
             return;
         } else {
-            if (!(p = lock_user(VERIFY_WRITE, ARG(1), len, 0))) {
+            p = lock_user(VERIFY_WRITE, arg1, len, 0);
+            if (!p) {
                 /* FIXME - check error code? */
                 result = -1;
             } else {
-                result = read(ARG(0), p, len);
-                unlock_user(p, ARG(1), len);
+                result = read(arg0, p, len);
+                unlock_user(p, arg1, len);
             }
         }
         break;
     case HOSTED_WRITE:
-        len = ARG(2);
+        GET_ARG(0);
+        GET_ARG(1);
+        GET_ARG(2);
+        len = arg2;
         if (use_gdb_syscalls()) {
             gdb_do_syscall(m68k_semi_cb, "write,%x,%x,%x",
-                           ARG(0), ARG(1), len);
+                           arg0, arg1, len);
             return;
         } else {
-            if (!(p = lock_user(VERIFY_READ, ARG(1), len, 1))) {
+            p = lock_user(VERIFY_READ, arg1, len, 1);
+            if (!p) {
                 /* FIXME - check error code? */
                 result = -1;
             } else {
-                result = write(ARG(0), p, len);
-                unlock_user(p, ARG(0), 0);
+                result = write(arg0, p, len);
+                unlock_user(p, arg0, 0);
             }
         }
         break;
     case HOSTED_LSEEK:
         {
             uint64_t off;
-            off = (uint32_t)ARG(2) | ((uint64_t)ARG(1) << 32);
+            GET_ARG(0);
+            GET_ARG(1);
+            GET_ARG(2);
+            GET_ARG(3);
+            off = (uint32_t)arg2 | ((uint64_t)arg1 << 32);
             if (use_gdb_syscalls()) {
                 m68k_semi_is_fseek = 1;
                 gdb_do_syscall(m68k_semi_cb, "fseek,%x,%lx,%x",
-                               ARG(0), off, ARG(3));
+                               arg0, off, arg3);
             } else {
-                off = lseek(ARG(0), off, ARG(3));
+                off = lseek(arg0, off, arg3);
                 /* FIXME - handle put_user() failure */
                 put_user_u32(off >> 32, args);
                 put_user_u32(off, args + 4);
@@ -255,74 +277,89 @@ void do_m68k_semihosting(CPUM68KState *env, int nr)
             return;
         }
     case HOSTED_RENAME:
+        GET_ARG(0);
+        GET_ARG(1);
+        GET_ARG(2);
+        GET_ARG(3);
         if (use_gdb_syscalls()) {
             gdb_do_syscall(m68k_semi_cb, "rename,%s,%s",
-                           ARG(0), (int)ARG(1), ARG(2), (int)ARG(3));
+                           arg0, (int)arg1, arg2, (int)arg3);
             return;
         } else {
-            p = lock_user_string(ARG(0));
-            q = lock_user_string(ARG(2));
+            p = lock_user_string(arg0);
+            q = lock_user_string(arg2);
             if (!p || !q) {
                 /* FIXME - check error code? */
                 result = -1;
             } else {
                 result = rename(p, q);
             }
-            unlock_user(p, ARG(0), 0);
-            unlock_user(q, ARG(2), 0);
+            unlock_user(p, arg0, 0);
+            unlock_user(q, arg2, 0);
         }
         break;
     case HOSTED_UNLINK:
+        GET_ARG(0);
+        GET_ARG(1);
         if (use_gdb_syscalls()) {
             gdb_do_syscall(m68k_semi_cb, "unlink,%s",
-                           ARG(0), (int)ARG(1));
+                           arg0, (int)arg1);
             return;
         } else {
-            if (!(p = lock_user_string(ARG(0)))) {
+            p = lock_user_string(arg0);
+            if (!p) {
                 /* FIXME - check error code? */
                 result = -1;
             } else {
                 result = unlink(p);
-                unlock_user(p, ARG(0), 0);
+                unlock_user(p, arg0, 0);
             }
         }
         break;
     case HOSTED_STAT:
+        GET_ARG(0);
+        GET_ARG(1);
+        GET_ARG(2);
         if (use_gdb_syscalls()) {
             gdb_do_syscall(m68k_semi_cb, "stat,%s,%x",
-                           ARG(0), (int)ARG(1), ARG(2));
+                           arg0, (int)arg1, arg2);
             return;
         } else {
             struct stat s;
-            if (!(p = lock_user_string(ARG(0)))) {
+            p = lock_user_string(arg0);
+            if (!p) {
                 /* FIXME - check error code? */
                 result = -1;
             } else {
                 result = stat(p, &s);
-                unlock_user(p, ARG(0), 0);
+                unlock_user(p, arg0, 0);
             }
             if (result == 0) {
-                translate_stat(env, ARG(2), &s);
+                translate_stat(env, arg2, &s);
             }
         }
         break;
     case HOSTED_FSTAT:
+        GET_ARG(0);
+        GET_ARG(1);
         if (use_gdb_syscalls()) {
             gdb_do_syscall(m68k_semi_cb, "fstat,%x,%x",
-                           ARG(0), ARG(1));
+                           arg0, arg1);
             return;
         } else {
             struct stat s;
-            result = fstat(ARG(0), &s);
+            result = fstat(arg0, &s);
             if (result == 0) {
-                translate_stat(env, ARG(1), &s);
+                translate_stat(env, arg1, &s);
             }
         }
         break;
     case HOSTED_GETTIMEOFDAY:
+        GET_ARG(0);
+        GET_ARG(1);
         if (use_gdb_syscalls()) {
             gdb_do_syscall(m68k_semi_cb, "gettimeofday,%x,%x",
-                           ARG(0), ARG(1));
+                           arg0, arg1);
             return;
         } else {
             qemu_timeval tv;
@@ -330,37 +367,41 @@ void do_m68k_semihosting(CPUM68KState *env, int nr)
             result = qemu_gettimeofday(&tv);
             if (result != 0) {
                 if (!(p = lock_user(VERIFY_WRITE,
-                                    ARG(0), sizeof(struct gdb_timeval), 0))) {
+                                    arg0, sizeof(struct gdb_timeval), 0))) {
                     /* FIXME - check error code? */
                     result = -1;
                 } else {
                     p->tv_sec = cpu_to_be32(tv.tv_sec);
                     p->tv_usec = cpu_to_be64(tv.tv_usec);
-                    unlock_user(p, ARG(0), sizeof(struct gdb_timeval));
+                    unlock_user(p, arg0, sizeof(struct gdb_timeval));
                 }
             }
         }
         break;
     case HOSTED_ISATTY:
+        GET_ARG(0);
         if (use_gdb_syscalls()) {
-            gdb_do_syscall(m68k_semi_cb, "isatty,%x", ARG(0));
+            gdb_do_syscall(m68k_semi_cb, "isatty,%x", arg0);
             return;
         } else {
-            result = isatty(ARG(0));
+            result = isatty(arg0);
         }
         break;
     case HOSTED_SYSTEM:
+        GET_ARG(0);
+        GET_ARG(1);
         if (use_gdb_syscalls()) {
             gdb_do_syscall(m68k_semi_cb, "system,%s",
-                           ARG(0), (int)ARG(1));
+                           arg0, (int)arg1);
             return;
         } else {
-            if (!(p = lock_user_string(ARG(0)))) {
+            p = lock_user_string(arg0);
+            if (!p) {
                 /* FIXME - check error code? */
                 result = -1;
             } else {
                 result = system(p);
-                unlock_user(p, ARG(0), 0);
+                unlock_user(p, arg0, 0);
             }
         }
         break;
@@ -402,6 +443,7 @@ void do_m68k_semihosting(CPUM68KState *env, int nr)
         cpu_abort(env, "Unsupported semihosting syscall %d\n", nr);
         result = 0;
     }
+failed:
     /* FIXME - handle put_user() failure */
     put_user_u32(result, args);
     put_user_u32(errno, args + 4);
-- 
1.7.11.4

  parent reply	other threads:[~2012-10-29 12:05 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-29 12:05 [Qemu-devel] [PATCH 0/3] target-m68k/m68k-semi: don't ignore get/put_user failure Peter Maydell
2012-10-29 12:05 ` [Qemu-devel] [PATCH 1/3] m68k: Return semihosting errno values correctly Peter Maydell
2012-10-29 12:05 ` Peter Maydell [this message]
2012-10-29 12:05 ` [Qemu-devel] [PATCH 3/3] target-m68k/m68k-semi.c: Log when put_user for returning values fails Peter Maydell
2012-11-03 12:51 ` [Qemu-devel] [PATCH 0/3] target-m68k/m68k-semi: don't ignore get/put_user failure Blue Swirl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1351512311-19106-3-git-send-email-peter.maydell@linaro.org \
    --to=peter.maydell@linaro.org \
    --cc=blauwirbel@gmail.com \
    --cc=meadori@codesourcery.com \
    --cc=patches@linaro.org \
    --cc=paul@codesourcery.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).