[Qemu-devel] [PATCH 2.2 v2 2/3] linux-user: Properly handle timer magic offset

[Alexander Graf]

When creating a timer handle, we give the timer id a special magic offset
of 0xcafe0000. However, we never mask that offset out of the timer id before
we start using it to dereference our timer array. So we always end up aborting
timer operations because the timer id is out of bounds.

This was not an issue before my patch e52a99f756e ("linux-user: Simplify
timerid checks on g_posix_timers range") because before we would blindly mask
anything above the first 16 bits.

This patch is superior to the plain masking in that it also adds validity checks
against the timer id to ensure we're always dealing with an actual timer id
created by QEMU.

Reported-by: Tom Musta <address@hidden>
Signed-off-by: Alexander Graf <address@hidden>

---

v1 -> v2:

  - Abort when magic is missing
---
 linux-user/syscall.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index d012c71..b7f12b6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9573,6 +9573,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
     }
 #endif
 
+#define TIMER_MAGIC 0xcafe0000
+#define TIMER_MAGIC_MASK 0xffff0000
+
 #ifdef TARGET_NR_timer_create
     case TARGET_NR_timer_create:
     {
@@ -9600,7 +9603,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
             if (ret) {
                 phtimer = NULL;
             } else {
-                if (put_user(0xcafe0000 | timer_index, arg3, target_timer_t)) {
+                if (put_user(TIMER_MAGIC | timer_index, arg3, target_timer_t)) 
{
                     goto efault;
                 }
             }
@@ -9616,6 +9619,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
          * struct itimerspec * old_value */
         target_ulong timerid = arg1;
 
+        /* Convert QEMU provided timer ID back to internal 16bit index format 
*/
+        if ((timerid & TIMER_MAGIC_MASK) == TIMER_MAGIC) {
+            timerid &= 0xffff;
+        } else {
+            ret = -TARGET_EINVAL;
+            break;
+        }
+
         if (arg3 == 0 || timerid >= ARRAY_SIZE(g_posix_timers)) {
             ret = -TARGET_EINVAL;
         } else {
@@ -9637,6 +9648,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
         /* args: timer_t timerid, struct itimerspec *curr_value */
         target_ulong timerid = arg1;
 
+        /* Convert QEMU provided timer ID back to internal 16bit index format 
*/
+        if ((timerid & TIMER_MAGIC_MASK) == TIMER_MAGIC) {
+            timerid &= 0xffff;
+        } else {
+            ret = -TARGET_EINVAL;
+            break;
+        }
+
         if (!arg2) {
             return -TARGET_EFAULT;
         } else if (timerid >= ARRAY_SIZE(g_posix_timers)) {
-- 
1.7.12.4