On 10 November 2014 20:33, Alexander Graf <ag...@suse.de> wrote:
> 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>
> @@ -9638,12 +9658,12 @@ 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 = get_timer_id(arg1);
>
> - if (!arg2) {
> - return -TARGET_EFAULT;
> - } else if (timerid >= ARRAY_SIZE(g_posix_timers)) {
> - ret = -TARGET_EINVAL;
> + if (timerid < 0) {
> + ret = timerid;
> + } else if (!arg2) {
> + ret = -TARGET_EFAULT;
This is changing the order of the checks so that we prefer EINVAL
over EFAULT if the caller passes in NULL arg2 and a bad ID; this
is in fact in accordance with what the kernel does, so it's correct.
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
thanks
-- PMM
