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 simplifies the code around timer id creation by introducing a proper target_timer_id typedef that is s32, just like Linux has it. It also changes the magic offset to a value that makes all timer ids be positive. Reported-by: Tom Musta <tommu...@gmail.com> Signed-off-by: Alexander Graf <ag...@suse.de> --- v1 -> v2: - Abort when magic is missing v2 -> v3: - Squash into a single patch - Change magic to always have positive IDs --- linux-user/syscall.c | 28 ++++++++++++++++++++++------ linux-user/syscall_defs.h | 5 +---- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index a175cc1..c21262f 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -9573,13 +9573,15 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, } #endif +#define TIMER_MAGIC 0x0caf0000 +#define TIMER_MAGIC_MASK 0xffff0000 + #ifdef TARGET_NR_timer_create case TARGET_NR_timer_create: { /* args: clockid_t clockid, struct sigevent *sevp, timer_t *timerid */ struct sigevent host_sevp = { {0}, }, *phost_sevp = NULL; - struct target_timer_t *ptarget_timer; int clkid = arg1; int timer_index = next_free_host_timer(); @@ -9601,11 +9603,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, if (ret) { phtimer = NULL; } else { - if (!lock_user_struct(VERIFY_WRITE, ptarget_timer, arg3, 1)) { + if (put_user(TIMER_MAGIC | timer_index, arg3, target_timer_t)) { goto efault; } - ptarget_timer->ptr = tswap32(0xcafe0000 | timer_index); - unlock_user_struct(ptarget_timer, arg3, 1); } } break; @@ -9617,7 +9617,15 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, { /* args: timer_t timerid, int flags, const struct itimerspec *new_value, * struct itimerspec * old_value */ - target_ulong timerid = arg1; + target_timer_t 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; @@ -9638,7 +9646,15 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, case TARGET_NR_timer_gettime: { /* args: timer_t timerid, struct itimerspec *curr_value */ - target_ulong timerid = arg1; + target_timer_t 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; diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h index c9e6323..ebb3be1 100644 --- a/linux-user/syscall_defs.h +++ b/linux-user/syscall_defs.h @@ -2564,10 +2564,7 @@ struct target_ucred { #endif - -struct target_timer_t { - abi_ulong ptr; -}; +typedef int32_t target_timer_t; #define TARGET_SIGEV_MAX_SIZE 64 -- 1.7.12.4